[PATCH] D36081: [X86] Improved X86::CMOV to Branch heuristic

[Zvi Rackover via Phabricator via llvm-commits]

zvi added inline comments.


================
Comment at: lib/Target/X86/X86CmovConversion.cpp:409
   //===--------------------------------------------------------------------===//
   bool WorthOptLoop = false;
+  if (Diff[1] < GainCycleThreshold)
----------------
Would this be a bit more readable?

```

if (diff[i] < GainCycleThreashold)
  return false;

bool WorthOptLoop = false;
if (Diff[1] == Diff[0])
    WorthOptLoop = Diff[0] * 8 >= LoopDepth[0].Depth;
else if (Diff[1] > Diff[0])
    WorthOptLoop =
        (Diff[1] - Diff[0]) * 2 >= (LoopDepth[1].Depth - LoopDepth[0].Depth) &&
        (Diff[1] * 8 >= LoopDepth[1].Depth);
```


================
Comment at: test/CodeGen/X86/pr33954.ll:1
+; RUN: llc -mtriple=x86_64-pc-linux -x86-cmov-converter=true -verify-machineinstrs < %s | FileCheck %s
+
----------------
Since the -x86-cmov-converter is invoked explicitly, should the  -x86-cmov-converter-threshold=4 flag be added too?


================
Comment at: test/CodeGen/X86/x86-cmov-converter.ll:29
 ;;
+;;   6. SmallGainPerLoop:
+;;        The gain from converting CMOV into branch is smaller than a threshold.
----------------
Maybe emphasize that in this case we bail out due to a low absolute gain in *cycles*, despite the *relative* gain being acceptable.


================
Comment at: test/CodeGen/X86/x86-cmov-converter.ll:99
+;;    int t = c[i];
+;;    if (c[i] * a > b)
+;;      t = 10;
----------------
if (t *a> b)


https://reviews.llvm.org/D36081