[PATCH] D49805: Fix corruption of result number in LegalizeVectorOps.cpp
Ulrich Weigand via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 25 09:02:57 PDT 2018
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>
More information about the llvm-commits
mailing list