From e714b98fff74a25097fd5a14c61ec5102fd4a1fc Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 11 Feb 2022 10:15:17 +0100 Subject: [PATCH] [InstCombine] Check type compatibility in indexed load fold This fold could use a rewrite to an offset-based implementation, but for now make sure it doesn't crash with opaque pointers. --- .../InstCombine/InstCombineCompares.cpp | 43 +++++++++---------- .../InstCombine/InstCombineInternal.h | 3 +- .../test/Transforms/InstCombine/opaque-ptr.ll | 39 +++++++++++++++++ 3 files changed, 62 insertions(+), 23 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp index caec4003f5b5..46af7d846892 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp @@ -105,10 +105,14 @@ static bool isSignTest(ICmpInst::Predicate &Pred, const APInt &C) { /// /// If AndCst is non-null, then the loaded value is masked with that constant /// before doing the comparison. This handles cases like "A[i]&4 == 0". -Instruction * -InstCombinerImpl::foldCmpLoadFromIndexedGlobal(GetElementPtrInst *GEP, - GlobalVariable *GV, CmpInst &ICI, - ConstantInt *AndCst) { +Instruction *InstCombinerImpl::foldCmpLoadFromIndexedGlobal( + LoadInst *LI, GetElementPtrInst *GEP, GlobalVariable *GV, CmpInst &ICI, + ConstantInt *AndCst) { + if (LI->isVolatile() || LI->getType() != GEP->getResultElementType() || + GV->getValueType() != GEP->getSourceElementType() || + !GV->isConstant() || !GV->hasDefinitiveInitializer()) + return nullptr; + Constant *Init = GV->getInitializer(); if (!isa(Init) && !isa(Init)) return nullptr; @@ -1865,15 +1869,13 @@ Instruction *InstCombinerImpl::foldICmpAndConstant(ICmpInst &Cmp, // Try to optimize things like "A[i] & 42 == 0" to index computations. Value *X = And->getOperand(0); Value *Y = And->getOperand(1); - if (auto *LI = dyn_cast(X)) - if (auto *GEP = dyn_cast(LI->getOperand(0))) - if (auto *GV = dyn_cast(GEP->getOperand(0))) - if (GV->isConstant() && GV->hasDefinitiveInitializer() && - !LI->isVolatile() && isa(Y)) { - ConstantInt *C2 = cast(Y); - if (Instruction *Res = foldCmpLoadFromIndexedGlobal(GEP, GV, Cmp, C2)) + if (auto *C2 = dyn_cast(Y)) + if (auto *LI = dyn_cast(X)) + if (auto *GEP = dyn_cast(LI->getOperand(0))) + if (auto *GV = dyn_cast(GEP->getOperand(0))) + if (Instruction *Res = + foldCmpLoadFromIndexedGlobal(LI, GEP, GV, Cmp, C2)) return Res; - } if (!Cmp.isEquality()) return nullptr; @@ -3476,13 +3478,11 @@ Instruction *InstCombinerImpl::foldICmpInstWithConstantNotInt(ICmpInst &I) { case Instruction::Load: // Try to optimize things like "A[i] > 4" to index computations. if (GetElementPtrInst *GEP = - dyn_cast(LHSI->getOperand(0))) { + dyn_cast(LHSI->getOperand(0))) if (GlobalVariable *GV = dyn_cast(GEP->getOperand(0))) - if (GV->isConstant() && GV->hasDefinitiveInitializer() && - !cast(LHSI)->isVolatile()) - if (Instruction *Res = foldCmpLoadFromIndexedGlobal(GEP, GV, I)) - return Res; - } + if (Instruction *Res = + foldCmpLoadFromIndexedGlobal(cast(LHSI), GEP, GV, I)) + return Res; break; } @@ -6632,10 +6632,9 @@ Instruction *InstCombinerImpl::visitFCmpInst(FCmpInst &I) { case Instruction::Load: if (auto *GEP = dyn_cast(LHSI->getOperand(0))) if (auto *GV = dyn_cast(GEP->getOperand(0))) - if (GV->isConstant() && GV->hasDefinitiveInitializer() && - !cast(LHSI)->isVolatile()) - if (Instruction *Res = foldCmpLoadFromIndexedGlobal(GEP, GV, I)) - return Res; + if (Instruction *Res = foldCmpLoadFromIndexedGlobal( + cast(LHSI), GEP, GV, I)) + return Res; break; } } diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h index c776ab6fbc2e..674d20461daf 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h +++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h @@ -652,7 +652,8 @@ public: ICmpInst::Predicate Cond, Instruction &I); Instruction *foldAllocaCmp(ICmpInst &ICI, const AllocaInst *Alloca, const Value *Other); - Instruction *foldCmpLoadFromIndexedGlobal(GetElementPtrInst *GEP, + Instruction *foldCmpLoadFromIndexedGlobal(LoadInst *LI, + GetElementPtrInst *GEP, GlobalVariable *GV, CmpInst &ICI, ConstantInt *AndCst = nullptr); Instruction *foldFCmpIntToFPConst(FCmpInst &I, Instruction *LHSI, diff --git a/llvm/test/Transforms/InstCombine/opaque-ptr.ll b/llvm/test/Transforms/InstCombine/opaque-ptr.ll index be715565bc5d..f054df6244e1 100644 --- a/llvm/test/Transforms/InstCombine/opaque-ptr.ll +++ b/llvm/test/Transforms/InstCombine/opaque-ptr.ll @@ -305,3 +305,42 @@ define i1 @cmp_gep_same_base_different_type(ptr %ptr, i64 %idx1, i64 %idx2) { %cmp = icmp ult ptr %gep1, %gep2 ret i1 %cmp } + +@ary = constant [4 x i8] [i8 1, i8 2, i8 3, i8 4] + +define i1 @cmp_load_gep_global(i64 %idx) { +; CHECK-LABEL: @cmp_load_gep_global( +; CHECK-NEXT: [[CMP:%.*]] = icmp eq i64 [[IDX:%.*]], 2 +; CHECK-NEXT: ret i1 [[CMP]] +; + %gep = getelementptr [4 x i8], ptr @ary, i64 0, i64 %idx + %load = load i8, ptr %gep + %cmp = icmp eq i8 %load, 3 + ret i1 %cmp +} + +define i1 @cmp_load_gep_global_different_load_type(i64 %idx) { +; CHECK-LABEL: @cmp_load_gep_global_different_load_type( +; CHECK-NEXT: [[GEP:%.*]] = getelementptr [4 x i8], ptr @ary, i64 0, i64 [[IDX:%.*]] +; CHECK-NEXT: [[LOAD:%.*]] = load i16, ptr [[GEP]], align 2 +; CHECK-NEXT: [[CMP:%.*]] = icmp eq i16 [[LOAD]], 3 +; CHECK-NEXT: ret i1 [[CMP]] +; + %gep = getelementptr [4 x i8], ptr @ary, i64 0, i64 %idx + %load = load i16, ptr %gep + %cmp = icmp eq i16 %load, 3 + ret i1 %cmp +} + +define i1 @cmp_load_gep_global_different_gep_type(i64 %idx) { +; CHECK-LABEL: @cmp_load_gep_global_different_gep_type( +; CHECK-NEXT: [[GEP:%.*]] = getelementptr [4 x i16], ptr @ary, i64 0, i64 [[IDX:%.*]] +; CHECK-NEXT: [[LOAD:%.*]] = load i16, ptr [[GEP]], align 2 +; CHECK-NEXT: [[CMP:%.*]] = icmp eq i16 [[LOAD]], 3 +; CHECK-NEXT: ret i1 [[CMP]] +; + %gep = getelementptr [4 x i16], ptr @ary, i64 0, i64 %idx + %load = load i16, ptr %gep + %cmp = icmp eq i16 %load, 3 + ret i1 %cmp +}