[llvm] be7d0d6 - [DAGCombiner] Fix issue with rot chain pattern

[via llvm-commits]

Author: chenglin.bi
Date: 2023-01-11T01:29:09+08:00
New Revision: be7d0d6db5094f2709c028ce6b62936f51db8bc9

URL: https://github.com/llvm/llvm-project/commit/be7d0d6db5094f2709c028ce6b62936f51db8bc9
DIFF: https://github.com/llvm/llvm-project/commit/be7d0d6db5094f2709c028ce6b62936f51db8bc9.diff

LOG: [DAGCombiner] Fix issue with rot chain pattern

faa35fc87370 fix the case of negative input shift. But when `c1`, `c2` is not the same side, it will also cause negative shift amount.
And that negative shift amount can't normalize by urem. So add one more bit size to normalize the last shift amount.

Fix: https://github.com/llvm/llvm-project/issues/59898

Reviewed By: RKSimon

Differential Revision: https://reviews.llvm.org/D141363

Added: 
    

Modified: 
    llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
    llvm/test/CodeGen/AArch64/rotate.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 78cbfe1356f76..0ec42d7306d43 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -9057,7 +9057,7 @@ SDValue DAGCombiner::visitRotate(SDNode *N) {
   unsigned NextOp = N0.getOpcode();
 
   // fold (rot* (rot* x, c2), c1)
-  //   -> (rot* x, ((c1 % bitsize) +- (c2 % bitsize)) % bitsize)
+  //   -> (rot* x, ((c1 % bitsize) +- (c2 % bitsize) + bitsize) % bitsize)
   if (NextOp == ISD::ROTL || NextOp == ISD::ROTR) {
     SDNode *C1 = DAG.isConstantIntBuildVectorOrConstantInt(N1);
     SDNode *C2 = DAG.isConstantIntBuildVectorOrConstantInt(N0.getOperand(1));
@@ -9073,6 +9073,8 @@ SDValue DAGCombiner::visitRotate(SDNode *N) {
       if (Norm1 && Norm2)
         if (SDValue CombinedShift = DAG.FoldConstantArithmetic(
                 CombineOp, dl, ShiftVT, {Norm1, Norm2})) {
+          CombinedShift = DAG.FoldConstantArithmetic(ISD::ADD, dl, ShiftVT,
+                                                     {CombinedShift, BitsizeC});
           SDValue CombinedShiftNorm = DAG.FoldConstantArithmetic(
               ISD::UREM, dl, ShiftVT, {CombinedShift, BitsizeC});
           return DAG.getNode(N->getOpcode(), dl, VT, N0->getOperand(0),

diff  --git a/llvm/test/CodeGen/AArch64/rotate.ll b/llvm/test/CodeGen/AArch64/rotate.ll
index c0623f841e547..a70bc70329bbe 100644
--- a/llvm/test/CodeGen/AArch64/rotate.ll
+++ b/llvm/test/CodeGen/AArch64/rotate.ll
@@ -18,14 +18,13 @@ define <2 x i64> @testcase(ptr %in) {
   ret <2 x i64> %4
 }
 
-;; FIXME: This causes miscompile because rot combine
+;; This causes miscompile because rot combine
 ;; doesn't handle negative shift well.
 define i5 @pr59898(i5 %x) {
 ; CHECK-LABEL: pr59898:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    lsr w8, w0, #4
-; CHECK-NEXT:    bfi w8, w0, #1, #31
-; CHECK-NEXT:    mov w0, w8
+; CHECK-NEXT:    ubfx w8, w0, #1, #4
+; CHECK-NEXT:    orr w0, w8, w0, lsl #4
 ; CHECK-NEXT:    ret
   %r1 = call i5 @llvm.fshr.i5(i5 %x, i5 %x, i5 3)
   %r2 = call i5 @llvm.fshl.i5(i5 %r1, i5 %r1, i5 2)