[PATCH] D71861: [LegalizeVectorOps] Pass the post-UpdateNodeOperands version of the Node to the LowerOperation/PromoteNode/ExpandNode calls

[Craig Topper via Phabricator via llvm-commits]

craig.topper created this revision.
craig.topper added reviewers: efriedma, RKSimon, spatel, arsenm.
Herald added subscribers: hiraditya, wdng.
Herald added a project: LLVM.

Its possible though very unlikely that the UpdateNodeOperands call that is made after legalizing operands will CSE to an existing node. When this happens we end up with Result pointing to a different node than Op. When this happens, in order to make sure the legalized operands propagate correctly, we need to use Result in the LowerOperation, ExpandNode, or PromoteNode calls made later.

While this fix works, I think this may result in Result being legalized twice because we aren't registering it in the LegalizedNodes map as itself. Maybe another option is to specifically detect this case and recursively call LegalizeOp on the node, then just call TranslateLegalizeResults to register it for Op?


https://reviews.llvm.org/D71861

Files:
  llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
  llvm/test/CodeGen/X86/avx512-cmp.ll


Index: llvm/test/CodeGen/X86/avx512-cmp.ll
===================================================================
--- llvm/test/CodeGen/X86/avx512-cmp.ll
+++ llvm/test/CodeGen/X86/avx512-cmp.ll
@@ -181,3 +181,39 @@
 if.end.i:
   ret i32 6
 }
+
+; This test previously caused an infinite loop in legalize vector ops. Due to
+; CSE triggering on the call to UpdateNodeOperands and the resulting node not
+; being passed to LowerOperation. The add is needed to force the zext into a
+; sext on that path. The shuffle keeps the zext alive. The xor somehow
+; influences the zext to be visited before the sext exposing the CSE opportunity
+; for the sext since zext of setcc is custom legalized to a sext and shift.
+define <8 x i32> @legalize_loop(<8 x double> %arg) {
+; KNL-LABEL: legalize_loop:
+; KNL:       ## %bb.0:
+; KNL-NEXT:    vxorpd %xmm1, %xmm1, %xmm1
+; KNL-NEXT:    vcmpnltpd %zmm0, %zmm1, %k1
+; KNL-NEXT:    vpternlogd $255, %zmm0, %zmm0, %zmm0 {%k1} {z}
+; KNL-NEXT:    vpsrld $31, %ymm0, %ymm1
+; KNL-NEXT:    vpshufd {{.*#+}} ymm1 = ymm1[3,2,1,0,7,6,5,4]
+; KNL-NEXT:    vpermq {{.*#+}} ymm1 = ymm1[2,3,0,1]
+; KNL-NEXT:    vpsubd %ymm0, %ymm1, %ymm0
+; KNL-NEXT:    retq
+;
+; SKX-LABEL: legalize_loop:
+; SKX:       ## %bb.0:
+; SKX-NEXT:    vxorpd %xmm1, %xmm1, %xmm1
+; SKX-NEXT:    vcmpnltpd %zmm0, %zmm1, %k0
+; SKX-NEXT:    vpmovm2d %k0, %ymm0
+; SKX-NEXT:    vpsrld $31, %ymm0, %ymm1
+; SKX-NEXT:    vpshufd {{.*#+}} ymm1 = ymm1[3,2,1,0,7,6,5,4]
+; SKX-NEXT:    vpermq {{.*#+}} ymm1 = ymm1[2,3,0,1]
+; SKX-NEXT:    vpsubd %ymm0, %ymm1, %ymm0
+; SKX-NEXT:    retq
+  %tmp = fcmp ogt <8 x double> %arg, zeroinitializer
+  %tmp1 = xor <8 x i1> %tmp, <i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true>
+  %tmp2 = zext <8 x i1> %tmp1 to <8 x i32>
+  %tmp3 = shufflevector <8 x i32> %tmp2, <8 x i32> undef, <8 x i32> <i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0>
+  %tmp4 = add <8 x i32> %tmp2, %tmp3
+  ret <8 x i32> %tmp4
+}
Index: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
+++ llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
@@ -464,7 +464,7 @@
   switch (Action) {
   default: llvm_unreachable("This action is not supported yet!");
   case TargetLowering::Promote:
-    Result = Promote(Op);
+    Result = Promote(Result);
     Changed = true;
     break;
   case TargetLowering::Legal:
@@ -472,7 +472,7 @@
     break;
   case TargetLowering::Custom: {
     LLVM_DEBUG(dbgs() << "Trying custom legalization\n");
-    if (SDValue Tmp1 = TLI.LowerOperation(Op, DAG)) {
+    if (SDValue Tmp1 = TLI.LowerOperation(Result, DAG)) {
       LLVM_DEBUG(dbgs() << "Successfully custom legalized node\n");
       Result = Tmp1;
       break;
@@ -481,7 +481,7 @@
     LLVM_FALLTHROUGH;
   }
   case TargetLowering::Expand:
-    Result = Expand(Op);
+    Result = Expand(Result);
   }
 
   // Make sure that the generated code is itself legal.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D71861.235234.patch
Type: text/x-patch
Size: 3021 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191224/da9860b6/attachment.bin>