[PATCH] D120332: [RISCV] Avoid infinite loop between DAGCombiner::visitMUL and RISCVISelLowering::transformAddImmMulImm

[Alex Bradbury via Phabricator via llvm-commits]

asb created this revision.
asb added reviewers: craig.topper, benshi001, luismarques.
Herald added subscribers: VincentWu, luke957, achieveartificialintelligence, StephenFan, vkmr, frasercrmck, evandro, apazos, sameer.abuasal, steven.zhang, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, hiraditya.
asb requested review of this revision.
Herald added subscribers: llvm-commits, pcwang-thead, eopXD, MaskRay.
Herald added a project: LLVM.

See https://github.com/llvm/llvm-project/issues/53831 for a full discussion.

The basic issue is that DAGCombiner::visitMUL and RISCVISelLowering;:transformAddImmMullImm get stuck in a loop, as the current checks in transformAddImmMulImm aren't sufficient to avoid all cases where DAGCombiner::isMulAddWithConstProfitable might trigger a transformation. This patch makes transformAddImmMulImm bail out if C0 (the constant used for multiplication) has more than one use.


https://reviews.llvm.org/D120332

Files:
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/test/CodeGen/RISCV/addimm-mulimm.ll


Index: llvm/test/CodeGen/RISCV/addimm-mulimm.ll
===================================================================
--- llvm/test/CodeGen/RISCV/addimm-mulimm.ll
+++ llvm/test/CodeGen/RISCV/addimm-mulimm.ll
@@ -872,3 +872,16 @@
   %tmp1 = add i64 %tmp0, -8990
   ret i64 %tmp1
 }
+
+; This test case previously caused an infinite loop between transformations
+; performed in RISCVISelLowering;:transformAddImmMulImm and
+; DAGCombiner::visitMUL.
+define i1 @pr53831(i32 %x) {
+  %tmp0 = add i32 %x, 1
+  %tmp1 = mul i32 %tmp0, 24
+  %tmp2 = add i32 %tmp1, 1
+  %tmp3 = mul i32 %x, 24
+  %tmp4 = add i32 %tmp3, 2048
+  %tmp5 = icmp eq i32 %tmp4, %tmp2
+	ret i1 %tmp5
+}
Index: llvm/lib/Target/RISCV/RISCVISelLowering.cpp
===================================================================
--- llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -7477,6 +7477,11 @@
   auto *N1C = dyn_cast<ConstantSDNode>(N->getOperand(1));
   if (!N0C || !N1C)
     return SDValue();
+  // If N0C has multiple uses it's possible one of the cases in
+  // DAGCombiner::isMulAddWithConstProfitable will be true, which would result
+  // in an infinite loop.
+  if (!N0C->hasOneUse())
+    return SDValue();
   int64_t C0 = N0C->getSExtValue();
   int64_t C1 = N1C->getSExtValue();
   int64_t CA, CB;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D120332.410546.patch
Type: text/x-patch
Size: 1323 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220222/6126d9d5/attachment.bin>