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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: test/CodeGen/X86/cmov-promotion.ll:155-158
+; CMOV-NEXT:    movl $117, %eax
+; CMOV-NEXT:    movl $237, %ecx
+; CMOV-NEXT:    cmovnel %eax, %ecx
+; CMOV-NEXT:    movsbl %cl, %eax
----------------
andreadb wrote:
> I noticed that we could avoid the sign extend if we move -19 instead of 237 to ECX, and we commute the operands of that CMOV (along with the condition: from NE to E).
> 
> The following sequence should be equivalent:
> 
> ```
> ; CMOV-NEXT:    movl $117, %eax
> ; CMOV-NEXT:    movl $-19, %ecx
> ; CMOV-NEXT:    cmovel %ecx, %eax
> ```
> 
> Same for other 'spromotion' tests below.
> 
> P.s.: none of these things require changes to your patch. This was just FIY (something that I found interesting while looking at the test changes).
Given that the input is
```
define i16 @old(i1 %c) {
  %t0 = select i1 %c, i8 117, i8 -19
  %ret = sext i8 %t0 to i16
  ret i16 %ret
}
```
why can't we simply widen the hands of select, like
```
define i16 @new(i1 %c) {
  %ret = select i1 %c, i16 117, i16 -19
  ret i16 %ret
}
```
https://rise4fun.com/Alive/cs8
?

So instead of 
```
        testb   $1, %dil
        movl    $117, %eax
        movl    $237, %ecx
        cmovnel %eax, %ecx
        movsbl  %cl, %eax

```
we get
```
        testb   $1, %dil
        movl    $117, %ecx
        movl    $65517, %eax            # imm = 0xFFED
        cmovnel %ecx, %eax
```

I.e. the 'obvious' fix here is that if we are widening result of CMOV,
and both of the possibilities are constants, then just widen those constants and CMOV itself.

Why do you think we'd need to flip the CMOV condition?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59035