[PATCH] D49805: Fix corruption of result number in LegalizeVectorOps.cpp

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand created this revision.
uweigand added reviewers: cameron.mcinally, craig.topper.
Herald added a subscriber: llvm-commits.

When VectorLegalizer::LegalizeOp creates a new SDValue after iterating over its arguments, it will always select the result number 0 even if the SDValue originally referred to some other result of the node.

This apparently never caused any issues so far (maybe nodes involving vector types with multiple results are rare?), but now with strict floating-point nodes it appears much more frequently.  I've noticed this in my own tests related to https://reviews.llvm.org/D45576, but it seems this is already visible with an in-tree test case that was recently checked in here: https://reviews.llvm.org/D48809

Looking at e.g. constrained_vector_fdiv_v4f64 in test/CodeGen/X86/vector-constrained-fp-intrinsics.ll, we get the following legalized DAG:

  t0: ch = EntryToken
  t19: v2f64 = BUILD_VECTOR ConstantFP:f64<1.000000e+01>, ConstantFP:f64<1.000000e+01>
    t21: v2f64 = BUILD_VECTOR ConstantFP:f64<3.000000e+00>, ConstantFP:f64<4.000000e+00>
  t23: v2f64,ch = strict_fdiv t0, t21, t19
    t20: v2f64 = BUILD_VECTOR ConstantFP:f64<1.000000e+00>, ConstantFP:f64<2.000000e+00>
  t22: v2f64,ch = strict_fdiv t0, t20, t19
    t24: ch = TokenFactor t22, t23

This is incorrect; note how the TokenFactor in t24 refers to arguments that aren't even chain values.  With this patch, we get instead the correct:

  t24: ch = TokenFactor t22:1, t23:1


Repository:
  rL LLVM

https://reviews.llvm.org/D49805

Files:
  lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
  test/CodeGen/X86/vector-constrained-fp-intrinsics.ll


Index: test/CodeGen/X86/vector-constrained-fp-intrinsics.ll
===================================================================
--- test/CodeGen/X86/vector-constrained-fp-intrinsics.ll
+++ test/CodeGen/X86/vector-constrained-fp-intrinsics.ll
@@ -27,10 +27,10 @@
 ; NO-FMA-LABEL: constrained_vector_fdiv_v4f64:
 ; NO-FMA:       # %bb.0:
 ; NO-FMA-NEXT:    movapd {{.*#+}} xmm2 = [1.000000e+01,1.000000e+01]
-; NO-FMA-NEXT:    movapd {{.*#+}} xmm1 = [3.000000e+00,4.000000e+00]
-; NO-FMA-NEXT:    divpd %xmm2, %xmm1
 ; NO-FMA-NEXT:    movapd {{.*#+}} xmm0 = [1.000000e+00,2.000000e+00]
 ; NO-FMA-NEXT:    divpd %xmm2, %xmm0
+; NO-FMA-NEXT:    movapd {{.*#+}} xmm1 = [3.000000e+00,4.000000e+00]
+; NO-FMA-NEXT:    divpd %xmm2, %xmm1
 ; NO-FMA-NEXT:    retq
 ;
 ; HAS-FMA-LABEL: constrained_vector_fdiv_v4f64:
@@ -72,10 +72,10 @@
 define <4 x double> @constrained_vector_fmul_v4f64() {
 ; NO-FMA-LABEL: constrained_vector_fmul_v4f64:
 ; NO-FMA:       # %bb.0: # %entry
-; NO-FMA-NEXT:    movapd {{.*#+}} xmm0 = [1.797693e+308,1.797693e+308]
-; NO-FMA-NEXT:    movapd {{.*#+}} xmm1 = [4.000000e+00,5.000000e+00]
-; NO-FMA-NEXT:    mulpd %xmm0, %xmm1
-; NO-FMA-NEXT:    mulpd {{.*}}(%rip), %xmm0
+; NO-FMA-NEXT:    movapd {{.*#+}} xmm1 = [1.797693e+308,1.797693e+308]
+; NO-FMA-NEXT:    movapd {{.*#+}} xmm0 = [2.000000e+00,3.000000e+00]
+; NO-FMA-NEXT:    mulpd %xmm1, %xmm0
+; NO-FMA-NEXT:    mulpd {{.*}}(%rip), %xmm1
 ; NO-FMA-NEXT:    retq
 ;
 ; HAS-FMA-LABEL: constrained_vector_fmul_v4f64:
@@ -119,10 +119,10 @@
 define <4 x double> @constrained_vector_fadd_v4f64() {
 ; NO-FMA-LABEL: constrained_vector_fadd_v4f64:
 ; NO-FMA:       # %bb.0: # %entry
-; NO-FMA-NEXT:    movapd {{.*#+}} xmm0 = [1.797693e+308,1.797693e+308]
-; NO-FMA-NEXT:    movapd {{.*#+}} xmm1 = [2.000000e+00,2.000000e-01]
-; NO-FMA-NEXT:    addpd %xmm0, %xmm1
-; NO-FMA-NEXT:    addpd {{.*}}(%rip), %xmm0
+; NO-FMA-NEXT:    movapd {{.*#+}} xmm1 = [1.797693e+308,1.797693e+308]
+; NO-FMA-NEXT:    movapd {{.*#+}} xmm0 = [1.000000e+00,1.000000e-01]
+; NO-FMA-NEXT:    addpd %xmm1, %xmm0
+; NO-FMA-NEXT:    addpd {{.*}}(%rip), %xmm1
 ; NO-FMA-NEXT:    retq
 ;
 ; HAS-FMA-LABEL: constrained_vector_fadd_v4f64:
@@ -165,10 +165,10 @@
 define <4 x double> @constrained_vector_fsub_v4f64() {
 ; NO-FMA-LABEL: constrained_vector_fsub_v4f64:
 ; NO-FMA:       # %bb.0: # %entry
-; NO-FMA-NEXT:    movapd {{.*#+}} xmm0 = [-1.797693e+308,-1.797693e+308]
-; NO-FMA-NEXT:    movapd %xmm0, %xmm1
-; NO-FMA-NEXT:    subpd {{.*}}(%rip), %xmm1
+; NO-FMA-NEXT:    movapd {{.*#+}} xmm1 = [-1.797693e+308,-1.797693e+308]
+; NO-FMA-NEXT:    movapd %xmm1, %xmm0
 ; NO-FMA-NEXT:    subpd {{.*}}(%rip), %xmm0
+; NO-FMA-NEXT:    subpd {{.*}}(%rip), %xmm1
 ; NO-FMA-NEXT:    retq
 ;
 ; HAS-FMA-LABEL: constrained_vector_fsub_v4f64:
@@ -425,8 +425,8 @@
 define <4 x double> @constrained_vector_sqrt_v4f64() {
 ; NO-FMA-LABEL: constrained_vector_sqrt_v4f64:
 ; NO-FMA:       # %bb.0: # %entry
-; NO-FMA-NEXT:    sqrtpd {{.*}}(%rip), %xmm1
 ; NO-FMA-NEXT:    sqrtpd {{.*}}(%rip), %xmm0
+; NO-FMA-NEXT:    sqrtpd {{.*}}(%rip), %xmm1
 ; NO-FMA-NEXT:    retq
 ;
 ; HAS-FMA-LABEL: constrained_vector_sqrt_v4f64:
Index: lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
+++ lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
@@ -223,7 +223,8 @@
   for (const SDValue &Op : Node->op_values())
     Ops.push_back(LegalizeOp(Op));
 
-  SDValue Result = SDValue(DAG.UpdateNodeOperands(Op.getNode(), Ops), 0);
+  SDValue Result = SDValue(DAG.UpdateNodeOperands(Op.getNode(), Ops),
+                           Op.getResNo());
 
   bool HasVectorValue = false;
   if (Op.getOpcode() == ISD::LOAD) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D49805.157285.patch
Type: text/x-patch
Size: 3740 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180725/176d76ca/attachment.bin>