Skip to content

Commit

Permalink
[InstSimplify] Don't lose inbounds when simplifying a GEP
Browse files Browse the repository at this point in the history
I noticed this while working on a (ptrtoint (gep null, x)) -> x fold.

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D110168
  • Loading branch information
arichardson committed Sep 23, 2021
1 parent 8b87c3d commit 05663dc
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 17 deletions.
2 changes: 1 addition & 1 deletion llvm/include/llvm/Analysis/InstructionSimplify.h
Expand Up @@ -248,7 +248,7 @@ Value *SimplifySelectInst(Value *Cond, Value *TrueVal, Value *FalseVal,
const SimplifyQuery &Q);

/// Given operands for a GetElementPtrInst, fold the result or return null.
Value *SimplifyGEPInst(Type *SrcTy, ArrayRef<Value *> Ops,
Value *SimplifyGEPInst(Type *SrcTy, ArrayRef<Value *> Ops, bool InBounds,
const SimplifyQuery &Q);

/// Given operands for an InsertValueInst, fold the result or return null.
Expand Down
20 changes: 11 additions & 9 deletions llvm/lib/Analysis/InstructionSimplify.cpp
Expand Up @@ -70,8 +70,8 @@ static Value *SimplifyOrInst(Value *, Value *, const SimplifyQuery &, unsigned);
static Value *SimplifyXorInst(Value *, Value *, const SimplifyQuery &, unsigned);
static Value *SimplifyCastInst(unsigned, Value *, Type *,
const SimplifyQuery &, unsigned);
static Value *SimplifyGEPInst(Type *, ArrayRef<Value *>, const SimplifyQuery &,
unsigned);
static Value *SimplifyGEPInst(Type *, ArrayRef<Value *>, bool,
const SimplifyQuery &, unsigned);
static Value *SimplifySelectInst(Value *, Value *, Value *,
const SimplifyQuery &, unsigned);

Expand Down Expand Up @@ -3983,7 +3983,8 @@ static Value *simplifyWithOpReplaced(Value *V, Value *Op, Value *RepOp,

if (auto *GEP = dyn_cast<GetElementPtrInst>(I))
return PreventSelfSimplify(SimplifyGEPInst(GEP->getSourceElementType(),
NewOps, Q, MaxRecurse - 1));
NewOps, GEP->isInBounds(), Q,
MaxRecurse - 1));

if (isa<SelectInst>(I))
return PreventSelfSimplify(
Expand Down Expand Up @@ -4328,7 +4329,7 @@ Value *llvm::SimplifySelectInst(Value *Cond, Value *TrueVal, Value *FalseVal,

/// Given operands for an GetElementPtrInst, see if we can fold the result.
/// If not, this returns null.
static Value *SimplifyGEPInst(Type *SrcTy, ArrayRef<Value *> Ops,
static Value *SimplifyGEPInst(Type *SrcTy, ArrayRef<Value *> Ops, bool InBounds,
const SimplifyQuery &Q, unsigned) {
// The type of the GEP pointer operand.
unsigned AS =
Expand Down Expand Up @@ -4448,13 +4449,13 @@ static Value *SimplifyGEPInst(Type *SrcTy, ArrayRef<Value *> Ops,
return nullptr;

auto *CE = ConstantExpr::getGetElementPtr(SrcTy, cast<Constant>(Ops[0]),
Ops.slice(1));
Ops.slice(1), InBounds);
return ConstantFoldConstant(CE, Q.DL);
}

Value *llvm::SimplifyGEPInst(Type *SrcTy, ArrayRef<Value *> Ops,
Value *llvm::SimplifyGEPInst(Type *SrcTy, ArrayRef<Value *> Ops, bool InBounds,
const SimplifyQuery &Q) {
return ::SimplifyGEPInst(SrcTy, Ops, Q, RecursionLimit);
return ::SimplifyGEPInst(SrcTy, Ops, InBounds, Q, RecursionLimit);
}

/// Given operands for an InsertValueInst, see if we can fold the result.
Expand Down Expand Up @@ -6220,8 +6221,9 @@ static Value *simplifyInstructionWithOperands(Instruction *I,
Result = SimplifySelectInst(NewOps[0], NewOps[1], NewOps[2], Q);
break;
case Instruction::GetElementPtr: {
Result = SimplifyGEPInst(cast<GetElementPtrInst>(I)->getSourceElementType(),
NewOps, Q);
auto *GEPI = cast<GetElementPtrInst>(I);
Result = SimplifyGEPInst(GEPI->getSourceElementType(), NewOps,
GEPI->isInBounds(), Q);
break;
}
case Instruction::InsertValue: {
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Analysis/PHITransAddr.cpp
Expand Up @@ -226,8 +226,8 @@ Value *PHITransAddr::PHITranslateSubExpr(Value *V, BasicBlock *CurBB,
return GEP;

// Simplify the GEP to handle 'gep x, 0' -> x etc.
if (Value *V = SimplifyGEPInst(GEP->getSourceElementType(),
GEPOps, {DL, TLI, DT, AC})) {
if (Value *V = SimplifyGEPInst(GEP->getSourceElementType(), GEPOps,
GEP->isInBounds(), {DL, TLI, DT, AC})) {
for (unsigned i = 0, e = GEPOps.size(); i != e; ++i)
RemoveInstInputs(GEPOps[i], InstInputs);

Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
Expand Up @@ -1855,7 +1855,8 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
Type *GEPType = GEP.getType();
Type *GEPEltType = GEP.getSourceElementType();
bool IsGEPSrcEleScalable = isa<ScalableVectorType>(GEPEltType);
if (Value *V = SimplifyGEPInst(GEPEltType, Ops, SQ.getWithInstruction(&GEP)))
if (Value *V = SimplifyGEPInst(GEPEltType, Ops, GEP.isInBounds(),
SQ.getWithInstruction(&GEP)))
return replaceInstUsesWith(GEP, V);

// For vector geps, use the generic demanded vector support.
Expand Down
7 changes: 4 additions & 3 deletions llvm/lib/Transforms/Scalar/NewGVN.cpp
Expand Up @@ -1194,9 +1194,10 @@ NewGVN::ExprResult NewGVN::createExpression(Instruction *I) const {
SimplifyCastInst(CI->getOpcode(), E->getOperand(0), CI->getType(), SQ);
if (auto Simplified = checkExprResults(E, I, V))
return Simplified;
} else if (isa<GetElementPtrInst>(I)) {
Value *V = SimplifyGEPInst(
E->getType(), ArrayRef<Value *>(E->op_begin(), E->op_end()), SQ);
} else if (auto *GEPI = dyn_cast<GetElementPtrInst>(I)) {
Value *V = SimplifyGEPInst(GEPI->getSourceElementType(),
ArrayRef<Value *>(E->op_begin(), E->op_end()),
GEPI->isInBounds(), SQ);
if (auto Simplified = checkExprResults(E, I, V))
return Simplified;
} else if (AllConstant) {
Expand Down
Expand Up @@ -28,7 +28,7 @@ entry:
}

; CHECK-LABEL: define void @f1
; CHECK: %1 = load i32 (i8 addrspace(4)*, i32 addrspace(1)*)*, i32 (i8 addrspace(4)*, i32 addrspace(1)*)* addrspace(4)* bitcast (i8* addrspace(4)* getelementptr (%__aaa_struct, %__aaa_struct addrspace(4)* addrspacecast (%__aaa_struct addrspace(1)* @__aaa_struct_ptr to %__aaa_struct addrspace(4)*), i64 0, i32 0, i32 3) to i32 (i8 addrspace(4)*, i32 addrspace(1)*)* addrspace(4)*), align 8
; CHECK: %1 = load i32 (i8 addrspace(4)*, i32 addrspace(1)*)*, i32 (i8 addrspace(4)*, i32 addrspace(1)*)* addrspace(4)* bitcast (i8* addrspace(4)* getelementptr inbounds (%__aaa_struct, %__aaa_struct addrspace(4)* addrspacecast (%__aaa_struct addrspace(1)* @__aaa_struct_ptr to %__aaa_struct addrspace(4)*), i64 0, i32 0, i32 3) to i32 (i8 addrspace(4)*, i32 addrspace(1)*)* addrspace(4)*), align 8

; Function Attrs: alwaysinline nounwind
define void @f1(i32 addrspace(1)*) #1 {
Expand Down

0 comments on commit 05663dc

Please sign in to comment.