[PATCH] D59035: [X86] Promote i8 CMOV's (PR40965)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:20550-20555
+  // Or finally, promote i8/i16 cmovs if it won't prevent folding a load.
+  // FIXME: we should not limit promotion of i8 case to only when the CMOV is
+  //        legal, but EmitLoweredSelect() can not deail with these extensions
+  //        being inserted between two CMOV's. (in i16 case too TBN)
+  //        https://bugs.llvm.org/show_bug.cgi?id=40974
+  if (((Op.getValueType() == MVT::i8 && Subtarget.hasCMov()) ||
----------------
I have looked into this a bit more, hacked together a patch D59147 that resolves the regression seen in D59001,
but it is unable to resolve the problem illustrated by possible `pseudo_cmov_lower.ll`, `@foo9` regression.
```
bb.2.entry:
; predecessors: %bb.0, %bb.1
  successors: %bb.3(0x40000000), %bb.4(0x40000000); %bb.3(50.00%), %bb.4(50.00%)
  liveins: $eflags
  %484:gr32 = PHI %483:gr32, %bb.1, %336:gr32, %bb.0    // <- base PHI
  %485:gr32_abcd = COPY %484:gr32
  %486:gr8 = COPY %485.sub_8bit:gr32_abcd
  %487:gr32 = MOVZX32rr8 killed %368:gr8                // <- def %487:gr32
  JA_1 %bb.4, implicit $eflags

bb.3.entry:
; predecessors: %bb.2
  successors: %bb.4(0x80000000); %bb.4(100.00%)
  liveins: $eflags

bb.4.entry:
; predecessors: %bb.2, %bb.3
  successors: %bb.5(0x40000000), %bb.6(0x40000000); %bb.5(50.00%), %bb.6(50.00%)
  liveins: $eflags
  %488:gr32 = PHI %487:gr32, %bb.3, %339:gr32, %bb.2    // <- second PHI of chain // <- use %487:gr32
  %489:gr32_abcd = COPY %488:gr32
  %490:gr8 = COPY %489.sub_8bit:gr32_abcd
  %491:gr32 = MOVZX32rr8 killed %367:gr8
  JA_1 %bb.6, implicit $eflags
```
`%488:gr32 = PHI` uses `%487:gr32` which is defined *after* the original `%484:gr32 = PHI`.
Some extra sinking/hoisting would be required.
Or, like @craig.topper mentioned in IRC, just promote all the i8's!!!1 :)

TLDR: if this `Subtarget.hasCMov()` limitation here is a problem, please advice how to proceed.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59035