[PATCH] D149782: [DAGCombiner] Add bswap(logic_op(bswap(x), y)) regression test case; NFC

[Austin Chang via Phabricator via llvm-commits]

austin880625 added inline comments.


================
Comment at: llvm/test/CodeGen/X86/combine-bswap.ll:385
+}
+
 ; negative test
----------------
RKSimon wrote:
> goldstein.w.n wrote:
> > goldstein.w.n wrote:
> > > Can you do two things.
> > > 
> > > 1) Add a negative test that is `(bswap (logic (bitreverse a), b))`
> > > 2) Make some of the tests use `bitreverse`. You don't need to add new tests, just modify half the `bswap` ones to use `bitreverse instead.
> > ping on this comment, otherwise LG.
> +1 for (1) - but I'd prefer equivalent tests added to combine-bitreverse.ll instead of being added to combine-bswap.ll
I was trying to add `(bitreverse (logic (bswap a), b))` in combine-bitreverse.ll and `(bswap (logic (bitreverse a), b))` in combine-bswap.ll and found the following:

x86 doesn't have RBIT like ARM, so the first instruction of `bitreverse` is a `bswap`(to replace 0x00FF00FF), then the `0x0F0F0F0F, 0x33333333,...` twiddling. This cause the `(bitreverse (logic (bswap a), b))` actually got optimized as a negative test, just not on the major part of `bitreverse`.

```
; X86-LABEL: brev_xor_rhs_bs32:                                                      
; X86:       # %bb.0:                                                                
; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
; X86-NEXT:    bswapl %eax ; <--- this becomes LHS
; X86-NEXT:    xorl {{[0-9]+}}(%esp), %eax
-; X86-NEXT:    bswapl %eax <---- this line got removed
; X86-NEXT:    movl %eax, %ecx
; X86-NEXT:    andl $252645135, %ecx # imm = 0xF0F0F0F
; X86-NEXT:    shll $4, %ecx
...
```

Currently I only keep `(bswap (logic (bitreverse a), b))` for this patch, then there's no diff after the optimization as desired, though this assumes the order of bswap and the manual bit ops.

Another way is moving these test cases into ARM version(need to create new files), which also reduces a lot of FileCheck lines


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

https://reviews.llvm.org/D149782