[PATCH] D48223: DAG combine "and|or (select c, -1, 0), x" -> "select c, x, 0|-1"

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

Sounds reasonable.



================
Comment at: test/CodeGen/X86/dagcombine-select.ll:3-17
+; CHECK-LABEL: {{^}}select_and1:
+; CHECK:      cmpl    $11, %edi
+; CHECK-NEXT: cmovgel %esi, %eax
+; CHECK-NEXT: retq
+define i32 @select_and1(i32 %x, i32 %y) {
+  %c = icmp slt i32 %x, 11
+  %s = select i1 %c, i32 0, i32 -1
----------------
rampitec wrote:
> lebedev.ri wrote:
> > rampitec wrote:
> > > rampitec wrote:
> > > > lebedev.ri wrote:
> > > > > Hm, is there some omitted instruction, or is this actually better than what we currently normally do?
> > > > > https://godbolt.org/g/7ULPfH
> > > > Yes. I have updated the test to contain the full ISA. First xor to zero out eax was omitted.
> > > > I am not sure what compiler explorer does, but that is what trunk llc has produced:
> > > > 
> > > > 
> > > > ```
> > > >         xorl    %eax, %eax
> > > >         cmpl    $11, %edi
> > > >         setl    %al
> > > >         decl    %eax
> > > >         andl    %esi, %eax
> > > >         retq
> > > > 
> > > > ```
> > > > 
> > > > I assume difference comes from running or not running opt.
> > > E.g. compare w/o opt: https://godbolt.org/g/NMZ9he
> > Ok, so the only difference is that the strictness of the comparison is inverted (`cmovg` vs `cmovge` and the other way around).
> Do you see "and eax, esi" instruction on the second from the right tab using my link? This one was eliminated.
> 
> In general if you run opt that is not an issue. If you run llc only (as in the test) you can see it. Normally that shall not happen, as I wrote in the description InstCombine can get rid of it. However, amdgcn can produce it during lowering and InstCombine does not play.
Of course.

I was only talking about comparing the `llc(D48223) test/CodeGen/X86/dagcombine-select.ll` with the `opt(trunk) test/CodeGen/X86/dagcombine-select.ll | llc(trunk)` output.


https://reviews.llvm.org/D48223