[PATCH] D59001: X86TargetLowering::LowerSELECT(): don't promote CMOV's if the subtarget does't have them

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: test/CodeGen/X86/midpoint-int.ll:803
+; X32-NEXT:    movl %edx, %edi
+; X32-NEXT:    ja .LBB11_2
+; X32-NEXT:  # %bb.1:
----------------
lebedev.ri wrote:
> craig.topper wrote:
> > Do you understand why we went from one conditional branch to two?
> I'm not sure yet, but i just want to point out that in all other cases we already had 2 conditional jumps.
> In this case we had one conditional jump and one uncondititonal jump, and that regressed to two conditional jumps.
Hmm, interesting.
```# *** IR Dump After Expand ISel Pseudo-instructions ***:
# Machine code for function scalar_i16_unsigned_reg_reg: IsSSA, TracksLiveness
Frame Objects:
  fi#-2: size=2, align=4, fixed, at location [SP+8]
  fi#-1: size=2, align=4, fixed, at location [SP+4]

bb.0 (%ir-block.0):
  successors: %bb.1(0x40000000), %bb.2(0x40000000); %bb.1(50.00%), %bb.2(50.00%)

  %0:gr32 = MOV32rm %fixed-stack.1, 1, $noreg, 0, $noreg :: (load 2 from %fixed-stack.1, align 4)
  %1:gr16 = COPY %0.sub_16bit:gr32
  %2:gr16 = MOV16rm %fixed-stack.0, 1, $noreg, 0, $noreg :: (load 2 from %fixed-stack.0, align 4)
  %3:gr16 = SUB16rr %1:gr16(tied-def 0), %2:gr16, implicit-def $eflags
  %4:gr8 = SETBEr implicit $eflags
  %5:gr32_nosp = MOVZX32rr8 killed %4:gr8
  %6:gr32 = LEA32r %5:gr32_nosp, 1, %5:gr32_nosp, -1, $noreg
  JA_1 %bb.2, implicit $eflags

bb.1 (%ir-block.0):
; predecessors: %bb.0
  successors: %bb.2(0x80000000); %bb.2(100.00%)
  liveins: $eflags

bb.2 (%ir-block.0):
; predecessors: %bb.0, %bb.1
  successors: %bb.3(0x40000000), %bb.4(0x40000000); %bb.3(50.00%), %bb.4(50.00%)
  liveins: $eflags
  %7:gr16 = PHI %1:gr16, %bb.1, %2:gr16, %bb.0
  %9:gr32 = IMPLICIT_DEF
  %8:gr32 = INSERT_SUBREG %9:gr32(tied-def 0), killed %7:gr16, %subreg.sub_16bit
  JA_1 %bb.4, implicit $eflags

bb.3 (%ir-block.0):
; predecessors: %bb.2
  successors: %bb.4(0x80000000); %bb.4(100.00%)


bb.4 (%ir-block.0):
; predecessors: %bb.2, %bb.3

  %10:gr16 = PHI %2:gr16, %bb.3, %1:gr16, %bb.2
  %12:gr32 = IMPLICIT_DEF
  %11:gr32 = INSERT_SUBREG %12:gr32(tied-def 0), killed %10:gr16, %subreg.sub_16bit
  %13:gr32 = SUB32rr %11:gr32(tied-def 0), killed %8:gr32, implicit-def dead $eflags
  %14:gr16 = COPY %13.sub_16bit:gr32
  %15:gr32 = MOVZX32rr16 killed %14:gr16
  %16:gr32 = SHR32r1 %15:gr32(tied-def 0), implicit-def dead $eflags
  %17:gr32 = IMUL32rr %16:gr32(tied-def 0), killed %6:gr32, implicit-def dead $eflags
  %18:gr32 = ADD32rr %17:gr32(tied-def 0), %0:gr32, implicit-def dead $eflags
  %19:gr16 = COPY %18.sub_16bit:gr32
  $ax = COPY %19:gr16
  RET 0, $ax

# End machine code for function scalar_i16_unsigned_reg_reg.
```
So we indeed have two conditional jumps based on the same condition.
But i'm not sure i understand.
 * `bb.4` is reachable from conditional jump from `bb.2` OR via a fallthrough from `bb.3`.
 * `bb.3` is only reachable via fallthrough from `bb.2`.
 * `bb.3` is empty
 * Thus, `bb.4` will always be visited if we visited `bb.2`.

This sounds wrong to me (i'm likely not getting something),
but why can't we just fold `bb.3` and `bb.4` into `bb.2`?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59001