[PATCH] D59758: [DAGCombiner] Combine OR as ADD when no common bits are set

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/test/CodeGen/X86/signbit-shift.ll:31-58
 define <4 x i32> @add_zext_ifpos_vec_splat(<4 x i32> %x) {
 ; CHECK-LABEL: add_zext_ifpos_vec_splat:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    pcmpeqd %xmm1, %xmm1
 ; CHECK-NEXT:    pcmpgtd %xmm1, %xmm0
-; CHECK-NEXT:    psrld $31, %xmm0
-; CHECK-NEXT:    por {{.*}}(%rip), %xmm0
+; CHECK-NEXT:    movdqa {{.*#+}} xmm1 = [42,42,42,42]
+; CHECK-NEXT:    psubd %xmm0, %xmm1
----------------
bjope wrote:
> lebedev.ri wrote:
> > Aren't these two tests identical?
> add_zext_ifpos_vec_splat2 is receiving %x in the second argument (so I guess it will be mapped to xmm1).
> 
> That second test was added to show that what seems to be a degradation in add_zext_ifpos_vec_splat happens due to register constraints (resulting in an extra movdqa at the end). Maybe I should omit the new test before commit. After all, it does not serve any extra purpose when it comes to testing "signbit-shift".
That extra move may be regalloc problem (or maybe that is already optimal).
The 'real' problem here is that we now have that extra `movdqa {{.*#+}} xmm1 = [42,42,42,42]`,
i guess because `psubd` can only take memory operand in other operand.
I'd drop the extra test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59758