From 046cf44c5719891cc17d151e8186a18549383446 Mon Sep 17 00:00:00 2001 From: Fabian Ritter Date: Thu, 18 Sep 2025 02:58:09 -0400 Subject: [PATCH] [SeparateConstOffsetFromGEP] Check if non-extracted indices may be negative when preserving inbounds If we know that the initial GEP was inbounds, and we change it to a sequence of GEPs from the same base pointer where every offset is non-negative, then the new GEPs are inbounds. So far, the implementation only checked if the extracted offsets are non-negative. In cases where non-extracted offsets can be negative, this would cause the inbounds flag to be wrongly preserved. Fixes an issue in #130617 found by nikic. --- .../Scalar/SeparateConstOffsetFromGEP.cpp | 13 +++++++------ .../AMDGPU/reorder-gep-inbounds.ll | 8 ++++---- .../SeparateConstOffsetFromGEP/NVPTX/gep-chain.ll | 6 +++--- .../SeparateConstOffsetFromGEP/NVPTX/split-gep.ll | 8 ++++---- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp index 8a5569743ab44..333cbb6ed1384 100644 --- a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp +++ b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp @@ -1116,22 +1116,23 @@ bool SeparateConstOffsetFromGEP::splitGEP(GetElementPtrInst *GEP) { // Splits this GEP index into a variadic part and a constant offset, and // uses the variadic part as the new index. - Value *OldIdx = GEP->getOperand(I); + Value *Idx = GEP->getOperand(I); User *UserChainTail; bool PreservesNUW; - Value *NewIdx = ConstantOffsetExtractor::Extract( - OldIdx, GEP, UserChainTail, PreservesNUW); + Value *NewIdx = ConstantOffsetExtractor::Extract(Idx, GEP, UserChainTail, + PreservesNUW); if (NewIdx != nullptr) { // Switches to the index with the constant offset removed. GEP->setOperand(I, NewIdx); // After switching to the new index, we can garbage-collect UserChain // and the old index if they are not used. RecursivelyDeleteTriviallyDeadInstructions(UserChainTail); - RecursivelyDeleteTriviallyDeadInstructions(OldIdx); - AllOffsetsNonNegative = - AllOffsetsNonNegative && isKnownNonNegative(NewIdx, *DL); + RecursivelyDeleteTriviallyDeadInstructions(Idx); + Idx = NewIdx; AllNUWPreserved &= PreservesNUW; } + AllOffsetsNonNegative = + AllOffsetsNonNegative && isKnownNonNegative(Idx, *DL); } } if (ExtractBase) { diff --git a/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/reorder-gep-inbounds.ll b/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/reorder-gep-inbounds.ll index 5cb3ee6f72e3b..f7c019bba5d1f 100644 --- a/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/reorder-gep-inbounds.ll +++ b/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/reorder-gep-inbounds.ll @@ -279,8 +279,8 @@ define void @addrspace3(ptr addrspace(3) %in.ptr, i64 %in.idx1) { ; CHECK-NEXT: entry: ; CHECK-NEXT: [[IN_IDX1_NNEG:%.*]] = and i64 [[IN_IDX1]], 9223372036854775807 ; CHECK-NEXT: [[IDXPROM:%.*]] = trunc i64 [[IN_IDX1_NNEG]] to i32 -; CHECK-NEXT: [[TMP0:%.*]] = getelementptr inbounds i128, ptr addrspace(3) [[IN_PTR]], i32 [[IDXPROM]] -; CHECK-NEXT: [[IDX11:%.*]] = getelementptr inbounds i8, ptr addrspace(3) [[TMP0]], i32 1024 +; CHECK-NEXT: [[TMP0:%.*]] = getelementptr i128, ptr addrspace(3) [[IN_PTR]], i32 [[IDXPROM]] +; CHECK-NEXT: [[IDX11:%.*]] = getelementptr i8, ptr addrspace(3) [[TMP0]], i32 1024 ; CHECK-NEXT: ret void ; entry: @@ -296,8 +296,8 @@ define void @addrspace7(ptr addrspace(7) %in.ptr, i64 %in.idx1) { ; CHECK-NEXT: entry: ; CHECK-NEXT: [[IN_IDX1_NNEG:%.*]] = and i64 [[IN_IDX1]], 9223372036854775807 ; CHECK-NEXT: [[IDXPROM:%.*]] = trunc i64 [[IN_IDX1_NNEG]] to i32 -; CHECK-NEXT: [[TMP0:%.*]] = getelementptr inbounds i128, ptr addrspace(7) [[IN_PTR]], i32 [[IDXPROM]] -; CHECK-NEXT: [[IDX11:%.*]] = getelementptr inbounds i8, ptr addrspace(7) [[TMP0]], i32 1024 +; CHECK-NEXT: [[TMP0:%.*]] = getelementptr i128, ptr addrspace(7) [[IN_PTR]], i32 [[IDXPROM]] +; CHECK-NEXT: [[IDX11:%.*]] = getelementptr i8, ptr addrspace(7) [[TMP0]], i32 1024 ; CHECK-NEXT: ret void ; entry: diff --git a/llvm/test/Transforms/SeparateConstOffsetFromGEP/NVPTX/gep-chain.ll b/llvm/test/Transforms/SeparateConstOffsetFromGEP/NVPTX/gep-chain.ll index e4b9dac72e96c..bbfab0f96fc53 100644 --- a/llvm/test/Transforms/SeparateConstOffsetFromGEP/NVPTX/gep-chain.ll +++ b/llvm/test/Transforms/SeparateConstOffsetFromGEP/NVPTX/gep-chain.ll @@ -24,11 +24,11 @@ define i32 @more_interesting(ptr %ptr, i32 %offset1, i32 %offset2) { ; CHECK-NEXT: [[IDXPROM:%.*]] = sext i32 [[OFFSET1]] to i64 ; CHECK-NEXT: [[GEP1:%.*]] = getelementptr inbounds [[STRUCT_UCHAR4:%.*]], ptr [[PTR]], i64 [[IDXPROM]] ; CHECK-NEXT: [[IDXPROM1:%.*]] = sext i32 [[OFFSET2]] to i64 -; CHECK-NEXT: [[TMP1:%.*]] = getelementptr inbounds [[STRUCT_UCHAR4]], ptr [[GEP1]], i64 [[IDXPROM1]] -; CHECK-NEXT: [[TMP2:%.*]] = getelementptr inbounds i8, ptr [[TMP1]], i64 8 +; CHECK-NEXT: [[TMP1:%.*]] = getelementptr [[STRUCT_UCHAR4]], ptr [[GEP1]], i64 [[IDXPROM1]] +; CHECK-NEXT: [[TMP2:%.*]] = getelementptr i8, ptr [[TMP1]], i64 8 ; CHECK-NEXT: [[V1:%.*]] = load i32, ptr [[TMP2]], align 4 ; CHECK-NEXT: [[IDXPROM2:%.*]] = sext i32 [[OFFSET2]] to i64 -; CHECK-NEXT: [[TMP4:%.*]] = getelementptr inbounds [[STRUCT_UCHAR4]], ptr [[TMP1]], i64 [[IDXPROM2]] +; CHECK-NEXT: [[TMP4:%.*]] = getelementptr [[STRUCT_UCHAR4]], ptr [[TMP1]], i64 [[IDXPROM2]] ; CHECK-NEXT: [[V2:%.*]] = load i32, ptr [[TMP4]], align 4 ; CHECK-NEXT: [[R:%.*]] = add i32 [[V1]], [[V2]] ; CHECK-NEXT: ret i32 [[R]] diff --git a/llvm/test/Transforms/SeparateConstOffsetFromGEP/NVPTX/split-gep.ll b/llvm/test/Transforms/SeparateConstOffsetFromGEP/NVPTX/split-gep.ll index da04a6e979425..77b3434f4f159 100644 --- a/llvm/test/Transforms/SeparateConstOffsetFromGEP/NVPTX/split-gep.ll +++ b/llvm/test/Transforms/SeparateConstOffsetFromGEP/NVPTX/split-gep.ll @@ -372,8 +372,8 @@ define ptr @trunk_explicit(ptr %ptr, i64 %idx) { ; CHECK-LABEL: define ptr @trunk_explicit( ; CHECK-SAME: ptr [[PTR:%.*]], i64 [[IDX:%.*]]) { ; CHECK-NEXT: entry: -; CHECK-NEXT: [[TMP0:%.*]] = getelementptr inbounds [[STRUCT0:%.*]], ptr [[PTR]], i64 0, i32 3, i64 [[IDX]], i32 1 -; CHECK-NEXT: [[PTR21:%.*]] = getelementptr inbounds i8, ptr [[TMP0]], i64 3216 +; CHECK-NEXT: [[TMP0:%.*]] = getelementptr [[STRUCT0:%.*]], ptr [[PTR]], i64 0, i32 3, i64 [[IDX]], i32 1 +; CHECK-NEXT: [[PTR21:%.*]] = getelementptr i8, ptr [[TMP0]], i64 3216 ; CHECK-NEXT: ret ptr [[PTR21]] ; entry: @@ -389,8 +389,8 @@ define ptr @trunk_long_idx(ptr %ptr, i64 %idx) { ; CHECK-LABEL: define ptr @trunk_long_idx( ; CHECK-SAME: ptr [[PTR:%.*]], i64 [[IDX:%.*]]) { ; CHECK-NEXT: entry: -; CHECK-NEXT: [[TMP0:%.*]] = getelementptr inbounds [[STRUCT0:%.*]], ptr [[PTR]], i64 0, i32 3, i64 [[IDX]], i32 1 -; CHECK-NEXT: [[PTR21:%.*]] = getelementptr inbounds i8, ptr [[TMP0]], i64 3216 +; CHECK-NEXT: [[TMP0:%.*]] = getelementptr [[STRUCT0:%.*]], ptr [[PTR]], i64 0, i32 3, i64 [[IDX]], i32 1 +; CHECK-NEXT: [[PTR21:%.*]] = getelementptr i8, ptr [[TMP0]], i64 3216 ; CHECK-NEXT: ret ptr [[PTR21]] ; entry: