Skip to content

Conversation

@sallto
Copy link
Contributor

@sallto sallto commented May 6, 2025

Partially fixes #138334
Combine fshl(X,X,Neg(Y)) into fshr(X,X,Y) and
fshr(X,X,Neg(Y)) into fshl(X,X,Y)

I opened this as a non-draft PR, as the second item in #138334 is not directly related to this change, therefore they can be seperate PRs.

@sallto sallto requested a review from nikic as a code owner May 6, 2025 21:03
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels May 6, 2025
@llvmbot
Copy link
Member

llvmbot commented May 6, 2025

@llvm/pr-subscribers-llvm-transforms

Author: None (sallto)

Changes

Partially fixes #138334
Combine fshl(X,X,Neg(Y)) into fshr(X,X,Y) and
fshr(X,X,Neg(Y)) into fshl(X,X,Y)

I opened this as a non-draft PR, as the second item in #138334 is not directly related to this change, therefore they can be seperate PRs.


Full diff: https://github.com/llvm/llvm-project/pull/138763.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp (+10)
  • (modified) llvm/test/Transforms/InstCombine/fsh.ll (+23)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index 844e18dd7d8c5..26e7b83f37ca8 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -2306,6 +2306,16 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
         return BitOp;
     }
 
+    // fshl(X, X, Neg(Y)) --> fshr(X, X, Y)
+    // fshr(X, X, Neg(Y)) --> fshl(X, X, Y)
+    Value *Y;
+    if (Op0 == Op1 && match(II->getArgOperand(2), m_Neg(m_Value(Y)))) {
+      Module *Mod = II->getModule();
+      Function *OppositeShift = Intrinsic::getOrInsertDeclaration(
+          Mod, IID == Intrinsic::fshl ? Intrinsic::fshr : Intrinsic::fshl, Ty);
+      return CallInst::Create(OppositeShift, {Op0, Op1, Y});
+    }
+
     // fshl(X, 0, Y) --> shl(X, and(Y, BitWidth - 1)) if bitwidth is a
     // power-of-2
     if (IID == Intrinsic::fshl && isPowerOf2_32(BitWidth) &&
diff --git a/llvm/test/Transforms/InstCombine/fsh.ll b/llvm/test/Transforms/InstCombine/fsh.ll
index 3ff4f9a2abf33..f05212bccaed8 100644
--- a/llvm/test/Transforms/InstCombine/fsh.ll
+++ b/llvm/test/Transforms/InstCombine/fsh.ll
@@ -1084,3 +1084,26 @@ define i8 @fshl_range_trunc(i1 %x) {
   %tr = trunc nsw i32 %fshl to i8
   ret i8 %tr
 }
+
+;; Issue #138334 negative rotate amounts can be folded into the opposite direction
+define i32 @fshl_neg_amount(i32 %x, i32 %y) {
+; CHECK-LABEL: define i32 @fshl_neg_amount(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]]) {
+; CHECK-NEXT:    [[R:%.*]] = call i32 @llvm.fshr.i32(i32 [[X]], i32 [[X]], i32 [[Y]])
+; CHECK-NEXT:    ret i32 [[R]]
+;
+  %n = sub i32 0, %y
+  %r = call i32 @llvm.fshl.i32(i32 %x, i32 %x, i32 %n)
+  ret i32 %r
+}
+
+define i32 @fshr_neg_amount(i32 %x, i32 %y) {
+; CHECK-LABEL: define i32 @fshr_neg_amount(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]]) {
+; CHECK-NEXT:    [[R:%.*]] = call i32 @llvm.fshl.i32(i32 [[X]], i32 [[X]], i32 [[Y]])
+; CHECK-NEXT:    ret i32 [[R]]
+;
+  %n = sub i32 0, %y
+  %r = call i32 @llvm.fshr.i32(i32 %x, i32 %x, i32 %n)
+  ret i32 %r
+}

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only correct for power of two bit widths.

i32: https://alive2.llvm.org/ce/z/6-ksXh
i31: https://alive2.llvm.org/ce/z/tJ7PAZ

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add a negative test where the funnel shift is not a rotate (operands not the same).

@sallto sallto force-pushed the funnel-shift-combine branch from a2b3cd8 to fafb193 Compare May 7, 2025 09:36
… direction (llvm#138334)

Combine fshl(X,X,Neg(Y)) into fshr(X,X,Y) and
fshr(X,X,Neg(Y)) into fshl(X,X,Y) if width is a power of two.

Confirmed here: https://alive2.llvm.org/ce/z/tX9iyE
@sallto sallto force-pushed the funnel-shift-combine branch from fafb193 to 36adce5 Compare May 7, 2025 09:41
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nikic nikic merged commit 1ee9576 into llvm:main May 7, 2025
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Funnel shift in 64-bit doesn't get matched to an intrinsic

3 participants