Skip to content

Conversation

@dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Jul 4, 2025

After #146100, the offset may be generated by a previous call to EmitGEPOffsets, causing the nuw flag on shl to be lost. This patch handles the shl+ptradd case as well. It also fixes a miscompilation in the case of mul + ptradd.
Alive2: https://alive2.llvm.org/ce/z/BeaNzE

This patch removes many unnecessary masking operations in Rust programs with the ptr_offset_from_unsigned intrinsic :
https://github.com/dtcxzyw/llvm-opt-benchmark/pull/2538/files

@dtcxzyw dtcxzyw requested a review from nikic as a code owner July 4, 2025 14:07
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels Jul 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 4, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

After #146100, the offset may be generated by a previous call to EmitGEPOffsets, causing the nuw flag on shl to be lost. This patch handles the shl+ptradd case as well. It also fixes a miscompilation in the case of mul + ptradd.
Alive2: https://alive2.llvm.org/ce/z/BeaNzE


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp (+6-3)
  • (modified) llvm/test/Transforms/InstCombine/sub-gep.ll (+52)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index f727eb0a63e05..8c16dfc82f5d5 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -2164,10 +2164,13 @@ Value *InstCombinerImpl::OptimizePointerDifference(Value *LHS, Value *RHS,
 
   // If this is a single inbounds GEP and the original sub was nuw,
   // then the final multiplication is also nuw.
-  if (auto *I = dyn_cast<Instruction>(Result))
+  if (auto *I = dyn_cast<OverflowingBinaryOperator>(Result))
     if (IsNUW && match(Offset2, m_Zero()) && Base.LHSNW.isInBounds() &&
-        I->getOpcode() == Instruction::Mul)
-      I->setHasNoUnsignedWrap();
+        I->hasNoSignedWrap() && !I->hasNoUnsignedWrap() &&
+        ((I->getOpcode() == Instruction::Mul &&
+          match(I->getOperand(1), m_NonNegative())) ||
+         I->getOpcode() == Instruction::Shl))
+      cast<Instruction>(I)->setHasNoUnsignedWrap();
 
   // If we have a 2nd GEP of the same base pointer, subtract the offsets.
   // If both GEPs are inbounds, then the subtract does not have signed overflow.
diff --git a/llvm/test/Transforms/InstCombine/sub-gep.ll b/llvm/test/Transforms/InstCombine/sub-gep.ll
index 993a06ad1780f..9a6c4c788159b 100644
--- a/llvm/test/Transforms/InstCombine/sub-gep.ll
+++ b/llvm/test/Transforms/InstCombine/sub-gep.ll
@@ -1089,3 +1089,55 @@ define <2 x i64> @splat_geps_multiple(ptr %base, i64 %idx0, <2 x i64> %idx1, <2
   %d = sub <2 x i64> %gep2.int, %gep1.int
   ret <2 x i64> %d
 }
+
+define i64 @nuw_ptrdiff_shl_nsw(ptr %base, i64 %idx) {
+; CHECK-LABEL: @nuw_ptrdiff_shl_nsw(
+; CHECK-NEXT:    [[OFFSET:%.*]] = shl nuw nsw i64 [[IDX:%.*]], 3
+; CHECK-NEXT:    ret i64 [[OFFSET]]
+;
+  %offset = shl nsw i64 %idx, 3
+  %gep = getelementptr inbounds i8, ptr %base, i64 %offset
+  %lhs = ptrtoint ptr %gep to i64
+  %rhs = ptrtoint ptr %base to i64
+  %diff = sub nuw i64 %lhs, %rhs
+  ret i64 %diff
+}
+
+define i64 @nuw_ptrdiff_shl_nonsw(ptr %base, i64 %idx) {
+; CHECK-LABEL: @nuw_ptrdiff_shl_nonsw(
+; CHECK-NEXT:    [[OFFSET:%.*]] = shl i64 [[IDX:%.*]], 3
+; CHECK-NEXT:    ret i64 [[OFFSET]]
+;
+  %offset = shl i64 %idx, 3
+  %gep = getelementptr inbounds i8, ptr %base, i64 %offset
+  %lhs = ptrtoint ptr %gep to i64
+  %rhs = ptrtoint ptr %base to i64
+  %diff = sub nuw i64 %lhs, %rhs
+  ret i64 %diff
+}
+
+define i64 @nuw_ptrdiff_mul_nsw_nneg_scale(ptr %base, i64 %idx) {
+; CHECK-LABEL: @nuw_ptrdiff_mul_nsw_nneg_scale(
+; CHECK-NEXT:    [[OFFSET:%.*]] = mul nuw nsw i64 [[IDX:%.*]], 3
+; CHECK-NEXT:    ret i64 [[OFFSET]]
+;
+  %offset = mul nsw i64 %idx, 3
+  %gep = getelementptr inbounds i8, ptr %base, i64 %offset
+  %lhs = ptrtoint ptr %gep to i64
+  %rhs = ptrtoint ptr %base to i64
+  %diff = sub nuw i64 %lhs, %rhs
+  ret i64 %diff
+}
+
+define i64 @nuw_ptrdiff_mul_nsw_unknown_scale(ptr %base, i64 %idx, i64 %scale) {
+; CHECK-LABEL: @nuw_ptrdiff_mul_nsw_unknown_scale(
+; CHECK-NEXT:    [[OFFSET:%.*]] = mul nsw i64 [[IDX:%.*]], [[SCALE:%.*]]
+; CHECK-NEXT:    ret i64 [[OFFSET]]
+;
+  %offset = mul nsw i64 %idx, %scale
+  %gep = getelementptr inbounds i8, ptr %base, i64 %offset
+  %lhs = ptrtoint ptr %gep to i64
+  %rhs = ptrtoint ptr %base to i64
+  %diff = sub nuw i64 %lhs, %rhs
+  ret i64 %diff
+}

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

@dtcxzyw dtcxzyw merged commit 2472cdc into llvm:main Jul 4, 2025
8 of 9 checks passed
@dtcxzyw dtcxzyw deleted the perf/nuw-ptr-diff branch July 4, 2025 15: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