[PATCH] D120233: [SelectOpti][5/5] Optimize select-to-branch transformation

[Sotiris Apostolakis via Phabricator via llvm-commits]

apostolakis added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectOptimize.cpp:398
+    // ILP with a naive ordering here.
+    SmallVector<Instruction *, 2> TrueSlicesInterleaved, FalseSlicesInterleaved;
+    for (unsigned long IS = 0; IS < maxTrueSliceLen; ++IS) {
----------------
davidxl wrote:
> Is it better to assert that TrueSlices and FalseSlices are not *both* empty? If that is that case, the transformation may be hurtful.
Sinking is an added optimization on top of the benefit of breaking data dependences. 
Even without any sinking, there are many cases where the transformation is still profitable.

For example, consider two dependence chains that include a load.
critical path:             .... -> load -> select -> .....
non-critical path                         \> store
The first one is a long chain and it is the critical path while the second one is a much shorter one.
The load cannot be sunk if the select is converted to a branch since it is used by another instruction (in the other path), but the conversion to a branch breaks a data dependence that reduces the critical path and thus it is still profitable. 



================
Comment at: llvm/test/CodeGen/X86/select-optimize.ll:152
 ; sink load
 define i32 @expensive_val_operand1(i32* nocapture %a, i32 %y, i1 %cmp) {
 ; CHECK-LABEL: @expensive_val_operand1(
----------------
davidxl wrote:
> It is better to add a test case showing sinking for SI group with more than one selects. 
Modified an existing test case with a two-select SI group to include sinking (and an instruction that shouldn't be sunk).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120233/new/

https://reviews.llvm.org/D120233