[PATCH] D56057: [X86] Individually simplify both operands of PMULDQ/PMULUDQ using the other entry point of SimplifyDemandedBits that allows the one use check of the root node to be suppressed.

[easyaspi314 (Devin) via Phabricator via llvm-commits]

easyaspi314 added a comment.

In D56057#1340502 <https://reviews.llvm.org/D56057#1340502>, @craig.topper wrote:

> Looking at the vector-reduce-mul.ll changes, I don't think this patch is valid. If the first node has multiple uses we can't propagate the input demanded bits to the next operation down.


Yeah, I checked it. It's definitely broken.

Although, @RKSimon, that is a perfect example of what I was saying, not checking tests. You almost accepted a patch that broke multivector 64-bit multiplication completely.

I transpiled the assembly from the test results and ran a diff, and sure enough, broken.

      .text
      .globl test_v4i64
  // long long test_v4i64(i64x2 xmm0, i64x2 xmm1)
  test_v4i64:
  #ifndef NEW
      movdqa %xmm0, %xmm2
      psrlq $32, %xmm2
      pmuludq %xmm1, %xmm2
      movdqa %xmm1, %xmm3
      psrlq $32, %xmm3
      pmuludq %xmm0, %xmm3
      paddq %xmm2, %xmm3
      psllq $32, %xmm3
  #endif
      pmuludq %xmm1, %xmm0
  #ifndef NEW
      paddq %xmm3, %xmm0
  #endif
      pshufd $78, %xmm0, %xmm1 # xmm1 = xmm0[2,3,0,1]
      movdqa %xmm0, %xmm2
      psrlq $32, %xmm2
      pmuludq %xmm1, %xmm2
      movdqa %xmm0, %xmm3
      psrldq $12, %xmm3
      pmuludq %xmm0, %xmm3
      paddq %xmm2, %xmm3
      psllq $32, %xmm3
      pmuludq %xmm1, %xmm0
      paddq %xmm3, %xmm0
      movq %xmm0, %rax
      retq



  #include <stdio.h>
  
  typedef long long i64x2 __attribute__((vector_size(16)));
  extern long long test_v4i64(i64x2 val1, i64x2 val2); // GCC passes a pointer if I emulate an i64x4
  
  int main(void) {
      i64x2 test = { 1234567812345678LL, 2345678923456789LL },
            test2 = { 1314151617181910LL, 9694959695969798LL };
       printf ("%lld\n", test_v4i64(test, test2));
  }

Compiled with GCC to avoid any LLVM-specific issues.

-UNEW: -7524135448842347520
-DNEW: 1393403030700022784


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56057