[Rpm-maint] [rpm-software-management/rpm] Use __builtin_cpu_supports in x86-64 CPU feature level detection (PR #2508)

[Panu Matilainen]

@pmatilai commented on this pull request.



>      const unsigned int op_1_ecx_lv2 = bit_SSE3 | bit_SSSE3 | bit_CMPXCHG16B | bit_SSE4_1 | bit_SSE4_2 | bit_POPCNT;
     if ((op_1_ecx & op_1_ecx_lv2) == op_1_ecx_lv2 && (op_80000001_ecx & bit_LAHF_LM))
 	level = 2;
 
     const unsigned int op_1_ecx_lv3 = bit_FMA | bit_MOVBE | bit_OSXSAVE | bit_AVX | bit_F16C;
     const unsigned int op_7_ebx_lv3 = bit_BMI | bit_AVX2 | bit_BMI2;
-    if (level == 2 && (op_1_ecx & op_1_ecx_lv3) == op_1_ecx_lv3 && (op_7_ebx & op_7_ebx_lv3) == op_7_ebx_lv3
-        && (op_80000001_ecx & bit_LZCNT))
+    if (level == 2 && (op_1_ecx & op_1_ecx_lv3) == op_1_ecx_lv3
+        && op_0_eax >= 7 && (op_7_ebx & op_7_ebx_lv3) == op_7_ebx_lv3
+        && (op_80000001_ecx & bit_LZCNT)
+        && __builtin_cpu_supports("avx2"))
         level = 3;

This is just cosmetics, but multiline if-conditions should be clearly separated from the following code-block. The && at front helps, but it's better (wrt rpm coding style) to either push the condition continuation to a clearly deeper indentation level, or use a brace on separate line, eg

```
if (level == 2 && (...
                   ...
                   ...))
     level = 3
```

or

```
if (level == 2 && (...
    ...
    ...))
{
    level = 3;
}
```

The latter often ends up being more readable, but depends on the case and individual taste.
Even more often, using meaningfully named helper variables to shorten the condition helps readability even more, but I wouldn't know where to even start here...

Applies to the other if touched in this patch too of course.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2508#pullrequestreview-1441028293
You are receiving this because you are subscribed to this thread.

Message ID: <rpm-software-management/rpm/pull/2508/review/1441028293 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20230523/c85657aa/attachment.html>