Skip to content

Commit

Permalink
Reapply [MergeICmps] Don't require GEP
Browse files Browse the repository at this point in the history
Recommit without changes over 53abe3f,
which addressed the cause of the reported crash.

-----

With opaque pointers, the zero-offset load will generally not use
a GEP. Allow a direct load without GEP, which is treated the same
way as a zero-offset GEP.
  • Loading branch information
nikic committed Mar 4, 2022
1 parent f9331c9 commit d3a5208
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 48 deletions.
52 changes: 32 additions & 20 deletions llvm/lib/Transforms/Scalar/MergeICmps.cpp
Expand Up @@ -144,31 +144,33 @@ BCEAtom visitICmpLoadOperand(Value *const Val, BaseIdentifier &BaseId) {
LLVM_DEBUG(dbgs() << "volatile or atomic\n");
return {};
}
Value *const Addr = LoadI->getOperand(0);
Value *Addr = LoadI->getOperand(0);
if (Addr->getType()->getPointerAddressSpace() != 0) {
LLVM_DEBUG(dbgs() << "from non-zero AddressSpace\n");
return {};
}
auto *const GEP = dyn_cast<GetElementPtrInst>(Addr);
if (!GEP)
return {};
LLVM_DEBUG(dbgs() << "GEP\n");
if (GEP->isUsedOutsideOfBlock(LoadI->getParent())) {
LLVM_DEBUG(dbgs() << "used outside of block\n");
return {};
}
const auto &DL = GEP->getModule()->getDataLayout();
if (!isDereferenceablePointer(GEP, LoadI->getType(), DL)) {
const auto &DL = LoadI->getModule()->getDataLayout();
if (!isDereferenceablePointer(Addr, LoadI->getType(), DL)) {
LLVM_DEBUG(dbgs() << "not dereferenceable\n");
// We need to make sure that we can do comparison in any order, so we
// require memory to be unconditionnally dereferencable.
return {};
}
APInt Offset = APInt(DL.getPointerTypeSizeInBits(GEP->getType()), 0);
if (!GEP->accumulateConstantOffset(DL, Offset))
return {};
return BCEAtom(GEP, LoadI, BaseId.getBaseId(GEP->getPointerOperand()),
Offset);

APInt Offset = APInt(DL.getPointerTypeSizeInBits(Addr->getType()), 0);
Value *Base = Addr;
auto *GEP = dyn_cast<GetElementPtrInst>(Addr);
if (GEP) {
LLVM_DEBUG(dbgs() << "GEP\n");
if (GEP->isUsedOutsideOfBlock(LoadI->getParent())) {
LLVM_DEBUG(dbgs() << "used outside of block\n");
return {};
}
if (!GEP->accumulateConstantOffset(DL, Offset))
return {};
Base = GEP->getPointerOperand();
}
return BCEAtom(GEP, LoadI, BaseId.getBaseId(Base), Offset);
}

// A comparison between two BCE atoms, e.g. `a == o.a` in the example at the
Expand Down Expand Up @@ -367,8 +369,11 @@ Optional<BCECmpBlock> visitCmpBlock(Value *const Val, BasicBlock *const Block,
return None;

BCECmpBlock::InstructionSet BlockInsts(
{Result->Lhs.GEP, Result->Rhs.GEP, Result->Lhs.LoadI, Result->Rhs.LoadI,
Result->CmpI, BranchI});
{Result->Lhs.LoadI, Result->Rhs.LoadI, Result->CmpI, BranchI});
if (Result->Lhs.GEP)
BlockInsts.insert(Result->Lhs.GEP);
if (Result->Rhs.GEP)
BlockInsts.insert(Result->Rhs.GEP);
return BCECmpBlock(std::move(*Result), Block, BlockInsts);
}

Expand Down Expand Up @@ -603,8 +608,15 @@ static BasicBlock *mergeComparisons(ArrayRef<BCECmpBlock> Comparisons,
NextCmpBlock->getParent(), InsertBefore);
IRBuilder<> Builder(BB);
// Add the GEPs from the first BCECmpBlock.
Value *const Lhs = Builder.Insert(FirstCmp.Lhs().GEP->clone());
Value *const Rhs = Builder.Insert(FirstCmp.Rhs().GEP->clone());
Value *Lhs, *Rhs;
if (FirstCmp.Lhs().GEP)
Lhs = Builder.Insert(FirstCmp.Lhs().GEP->clone());
else
Lhs = FirstCmp.Lhs().LoadI->getPointerOperand();
if (FirstCmp.Rhs().GEP)
Rhs = Builder.Insert(FirstCmp.Rhs().GEP->clone());
else
Rhs = FirstCmp.Rhs().LoadI->getPointerOperand();

Value *IsEqual = nullptr;
LLVM_DEBUG(dbgs() << "Merging " << Comparisons.size() << " comparisons -> "
Expand Down
19 changes: 5 additions & 14 deletions llvm/test/Transforms/MergeICmps/X86/no-gep-other-work.ll
Expand Up @@ -10,22 +10,13 @@ declare void @other_work()

define i1 @test(i8* dereferenceable(2) %arg, i8* dereferenceable(2) %arg1) {
; CHECK-LABEL: @test(
; CHECK-NEXT: entry:
; CHECK-NEXT: "if+entry":
; CHECK-NEXT: call void @other_work()
; CHECK-NEXT: [[ARG_OFF:%.*]] = getelementptr inbounds i8, i8* [[ARG:%.*]], i64 1
; CHECK-NEXT: [[ARG1_OFF:%.*]] = getelementptr inbounds i8, i8* [[ARG1:%.*]], i64 1
; CHECK-NEXT: [[ARG_OFF_VAL:%.*]] = load i8, i8* [[ARG_OFF]], align 1
; CHECK-NEXT: [[ARG1_OFF_VAL:%.*]] = load i8, i8* [[ARG1_OFF]], align 1
; CHECK-NEXT: [[CMP_OFF:%.*]] = icmp eq i8 [[ARG_OFF_VAL]], [[ARG1_OFF_VAL]]
; CHECK-NEXT: br i1 [[CMP_OFF]], label [[IF:%.*]], label [[JOIN:%.*]]
; CHECK: if:
; CHECK-NEXT: [[ARG_VAL:%.*]] = load i8, i8* [[ARG]], align 1
; CHECK-NEXT: [[ARG1_VAL:%.*]] = load i8, i8* [[ARG1]], align 1
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[ARG_VAL]], [[ARG1_VAL]]
; CHECK-NEXT: br label [[JOIN]]
; CHECK-NEXT: [[MEMCMP:%.*]] = call i32 @memcmp(i8* [[ARG:%.*]], i8* [[ARG1:%.*]], i64 2)
; CHECK-NEXT: [[TMP0:%.*]] = icmp eq i32 [[MEMCMP]], 0
; CHECK-NEXT: br label [[JOIN:%.*]]
; CHECK: join:
; CHECK-NEXT: [[PHI:%.*]] = phi i1 [ false, [[ENTRY:%.*]] ], [ [[CMP]], [[IF]] ]
; CHECK-NEXT: ret i1 [[PHI]]
; CHECK-NEXT: ret i1 [[TMP0]]
;
entry:
call void @other_work()
Expand Down
19 changes: 5 additions & 14 deletions llvm/test/Transforms/MergeICmps/X86/opaque-ptr.ll
Expand Up @@ -7,21 +7,12 @@ target triple = "x86_64-unknown-unknown"

define i1 @test(ptr dereferenceable(8) %a, ptr dereferenceable(8) %b) {
; CHECK-LABEL: @test(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[V0:%.*]] = load i32, ptr [[A:%.*]], align 4
; CHECK-NEXT: [[V1:%.*]] = load i32, ptr [[B:%.*]], align 4
; CHECK-NEXT: [[CMP_I:%.*]] = icmp eq i32 [[V0]], [[V1]]
; CHECK-NEXT: br i1 [[CMP_I]], label [[LAND_RHS_I:%.*]], label [[OPEQ1_EXIT:%.*]]
; CHECK: land.rhs.i:
; CHECK-NEXT: [[SECOND_I:%.*]] = getelementptr inbounds [[S:%.*]], ptr [[A]], i64 0, i32 1
; CHECK-NEXT: [[V2:%.*]] = load i32, ptr [[SECOND_I]], align 4
; CHECK-NEXT: [[SECOND2_I:%.*]] = getelementptr inbounds [[S]], ptr [[B]], i64 0, i32 1
; CHECK-NEXT: [[V3:%.*]] = load i32, ptr [[SECOND2_I]], align 4
; CHECK-NEXT: [[CMP3_I:%.*]] = icmp eq i32 [[V2]], [[V3]]
; CHECK-NEXT: br label [[OPEQ1_EXIT]]
; CHECK-NEXT: "entry+land.rhs.i":
; CHECK-NEXT: [[MEMCMP:%.*]] = call i32 @memcmp(ptr [[A:%.*]], ptr [[B:%.*]], i64 8)
; CHECK-NEXT: [[TMP0:%.*]] = icmp eq i32 [[MEMCMP]], 0
; CHECK-NEXT: br label [[OPEQ1_EXIT:%.*]]
; CHECK: opeq1.exit:
; CHECK-NEXT: [[PHI:%.*]] = phi i1 [ false, [[ENTRY:%.*]] ], [ [[CMP3_I]], [[LAND_RHS_I]] ]
; CHECK-NEXT: ret i1 [[PHI]]
; CHECK-NEXT: ret i1 [[TMP0]]
;
entry:
%v0 = load i32, ptr %a, align 4
Expand Down

0 comments on commit d3a5208

Please sign in to comment.