Skip to content

Conversation

@RKSimon
Copy link
Collaborator

@RKSimon RKSimon commented Mar 7, 2025

Minor tweak to #129363 which handled all the cases where there was a sext for the original source value, but not for cases where the source is already half the size of the destination type

Another regression noticed in #76524

…))), bw) -> (sext x) fold

Minor tweak llvm#129363 which handled all the cases where there was a sext for the original source value, but not for cases where the source is already half the size of the destination type

Another regression noticed in llvm#76524
@RKSimon RKSimon requested a review from dtcxzyw March 7, 2025 17:54
@RKSimon RKSimon requested a review from nikic as a code owner March 7, 2025 17:54
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels Mar 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 7, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Simon Pilgrim (RKSimon)

Changes

Minor tweak to #129363 which handled all the cases where there was a sext for the original source value, but not for cases where the source is already half the size of the destination type

Another regression noticed in #76524


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp (+3-4)
  • (modified) llvm/test/Transforms/InstCombine/iX-ext-split.ll (+1-5)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index 330358ee749e5..f81e2de7b365c 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -3122,13 +3122,12 @@ static Value *matchOrConcat(Instruction &Or, InstCombiner::BuilderTy &Builder) {
   // iX ext split: extending or(zext(x),shl(zext(y),bw/2) pattern
   // to consume sext/ashr: or(zext(sext(x)),shl(zext(sext(ashr(x))),bw/2)
   Value *X;
-  if (match(LowerSrc, m_SExt(m_Value(X))) &&
+  if (match(LowerSrc, m_SExtOrSelf(m_Value(X))) &&
       match(UpperSrc,
-            m_SExt(m_AShr(
+            m_SExtOrSelf(m_AShr(
                 m_Specific(X),
-                m_SpecificInt(X->getType()->getScalarSizeInBits() - 1))))) {
+                m_SpecificInt(X->getType()->getScalarSizeInBits() - 1)))))
     return Builder.CreateSExt(X, Ty);
-  }
 
   return nullptr;
 }
diff --git a/llvm/test/Transforms/InstCombine/iX-ext-split.ll b/llvm/test/Transforms/InstCombine/iX-ext-split.ll
index 5f8276934c2ef..fc804df0e4bec 100644
--- a/llvm/test/Transforms/InstCombine/iX-ext-split.ll
+++ b/llvm/test/Transforms/InstCombine/iX-ext-split.ll
@@ -94,11 +94,7 @@ define void @i128_ext_split_store_i64(i64 %x, ptr %out)  {
 ; CHECK-LABEL: define void @i128_ext_split_store_i64(
 ; CHECK-SAME: i64 [[X:%.*]], ptr [[OUT:%.*]]) {
 ; CHECK-NEXT:  [[ENTRY:.*:]]
-; CHECK-NEXT:    [[LO:%.*]] = zext i64 [[X]] to i128
-; CHECK-NEXT:    [[SIGN:%.*]] = ashr i64 [[X]], 63
-; CHECK-NEXT:    [[WIDEN:%.*]] = zext i64 [[SIGN]] to i128
-; CHECK-NEXT:    [[HI:%.*]] = shl nuw i128 [[WIDEN]], 64
-; CHECK-NEXT:    [[RES:%.*]] = or disjoint i128 [[HI]], [[LO]]
+; CHECK-NEXT:    [[RES:%.*]] = sext i64 [[X]] to i128
 ; CHECK-NEXT:    store i128 [[RES]], ptr [[OUT]], align 16
 ; CHECK-NEXT:    ret void
 ;

Copy link
Member

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

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

LGTM w/ some nits.

return ConcatIntrinsicCalls(Intrinsic::bitreverse, UpperBRev, LowerBRev);

// iX ext split: extending or(zext(x),shl(zext(y),bw/2) pattern
// to consume sext/ashr: or(zext(sext(x)),shl(zext(sext(ashr(x))),bw/2)
Copy link
Member

Choose a reason for hiding this comment

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

Comments need to be updated. BTW, bw/2 should be changed to bw-1.

@RKSimon RKSimon changed the title [InstCombine] Add handling for (or (zext x), (shl (zext (ashr x, bw-1))), bw) -> (sext x) fold [InstCombine] Add handling for (or (zext x), (shl (zext (ashr x, xbw-1))), bw) -> (sext x) fold Mar 8, 2025
@RKSimon RKSimon changed the title [InstCombine] Add handling for (or (zext x), (shl (zext (ashr x, xbw-1))), bw) -> (sext x) fold [InstCombine] Add handling for (or (zext x), (shl (zext (ashr x, xbw-1))), bw/2) -> (sext x) fold Mar 8, 2025
@RKSimon RKSimon changed the title [InstCombine] Add handling for (or (zext x), (shl (zext (ashr x, xbw-1))), bw/2) -> (sext x) fold [InstCombine] Add handling for (or (zext x), (shl (zext (ashr x, bw/2-1))), bw/2) -> (sext x) fold Mar 8, 2025
@RKSimon RKSimon merged commit d84dc8f into llvm:main Mar 9, 2025
9 of 11 checks passed
@RKSimon RKSimon deleted the ic-sext-split branch March 9, 2025 10:34
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.

3 participants