diff --git a/llvm/lib/Transforms/Scalar/MergeICmps.cpp b/llvm/lib/Transforms/Scalar/MergeICmps.cpp index 4c7c215d3cb3b..f8beaab7c6f5c 100644 --- a/llvm/lib/Transforms/Scalar/MergeICmps.cpp +++ b/llvm/lib/Transforms/Scalar/MergeICmps.cpp @@ -162,13 +162,6 @@ static BCEAtom visitICmpLoadOperand(Value *const Val, BaseIdentifier &BaseId) { return {}; } - 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 unconditionally dereferenceable. - return {}; - } - APInt Offset = APInt(DL.getIndexTypeSizeInBits(Addr->getType()), 0); Value *Base = Addr; auto *GEP = dyn_cast(Addr); @@ -225,7 +218,9 @@ class BCECmpBlock { // Returns true if the non-BCE-cmp instructions can be separated from BCE-cmp // instructions in the block. - bool canSplit(AliasAnalysis &AA) const; + // SplitAt is set to the instruction before which the block will have to be + // split. + bool canSplit(AliasAnalysis &AA, Instruction *&SplitAt) const; // Return true if this all the relevant instructions in the BCE-cmp-block can // be sunk below this instruction. By doing this, we know we can separate the @@ -290,9 +285,11 @@ void BCECmpBlock::split(BasicBlock *NewParent, AliasAnalysis &AA) const { Inst->moveBeforePreserving(*NewParent, NewParent->begin()); } -bool BCECmpBlock::canSplit(AliasAnalysis &AA) const { +bool BCECmpBlock::canSplit(AliasAnalysis &AA, Instruction *&SplitAt) const { + SplitAt = nullptr; for (Instruction &Inst : *BB) { if (!BlockInsts.count(&Inst)) { + SplitAt = Inst.getNextNode(); if (!canSinkBCECmpInst(&Inst, AA)) return false; } @@ -416,6 +413,8 @@ class BCECmpChain { BCECmpChain(const std::vector &Blocks, PHINode &Phi, AliasAnalysis &AA); + bool isDereferenceable(); + bool simplify(const TargetLibraryInfo &TLI, AliasAnalysis &AA, DomTreeUpdater &DTU); @@ -430,6 +429,9 @@ class BCECmpChain { std::vector MergedBlocks_; // The original entry block (before sorting); BasicBlock *EntryBlock_; + // The instruction before which the entry block needs to be split (or null + // if no splitting required). + Instruction *SplitAt = nullptr; }; } // namespace @@ -518,13 +520,14 @@ BCECmpChain::BCECmpChain(const std::vector &Blocks, PHINode &Phi, // and start anew. // // NOTE: we only handle blocks a with single predecessor for now. - if (Comparison->canSplit(AA)) { + if (Comparison->canSplit(AA, SplitAt)) { LLVM_DEBUG(dbgs() << "Split initial block '" << Comparison->BB->getName() << "' that does extra work besides compare\n"); Comparison->RequireSplit = true; enqueueBlock(Comparisons, std::move(*Comparison)); } else { + SplitAt = nullptr; LLVM_DEBUG(dbgs() << "ignoring initial block '" << Comparison->BB->getName() << "' that does extra work besides compare\n"); @@ -734,6 +737,37 @@ static BasicBlock *mergeComparisons(ArrayRef Comparisons, return BB; } +// The transform may change the order in which the comparison is performed, +// in which case we may perform loads that were not performed by the original +// program. As such, we need to ensure that all the accessed memory is +// dereferenceable. +bool BCECmpChain::isDereferenceable() { + // We know that there can be no frees inside the merged blocks, so it's + // sufficient for dereferenceability to hold at the entry block. One + // exception to this is if the entry block performs "other work" and will + // get split. In that case, we need to consider frees prior to the splitting + // point. + Instruction *CxtI = SplitAt ? SplitAt : &EntryBlock_->front(); + + for (const auto &Blocks : MergedBlocks_) { + const BCECmpBlock &LowestBlock = Blocks.front(); + const Value *Lhs = LowestBlock.Lhs().LoadI->getPointerOperand(); + const Value *Rhs = LowestBlock.Rhs().LoadI->getPointerOperand(); + const DataLayout &DL = LowestBlock.Lhs().LoadI->getDataLayout(); + + unsigned SizeInBits = 0; + for (const BCECmpBlock &Block : Blocks) + SizeInBits += Block.SizeBits(); + + APInt Size(64, SizeInBits / 8); + SimplifyQuery SQ(DL, CxtI); + if (!isDereferenceableAndAlignedPointer(Lhs, Align(1), Size, SQ) || + !isDereferenceableAndAlignedPointer(Rhs, Align(1), Size, SQ)) + return false; + } + return true; +} + bool BCECmpChain::simplify(const TargetLibraryInfo &TLI, AliasAnalysis &AA, DomTreeUpdater &DTU) { assert(atLeastOneMerged() && "simplifying trivial BCECmpChain"); @@ -888,6 +922,11 @@ static bool processPhi(PHINode &Phi, const TargetLibraryInfo &TLI, return false; } + if (!CmpChain.isDereferenceable()) { + LLVM_DEBUG(dbgs() << "not dereferenceable\n"); + return false; + } + return CmpChain.simplify(TLI, AA, DTU); } diff --git a/llvm/test/Transforms/MergeICmps/X86/no-gep-other-work.ll b/llvm/test/Transforms/MergeICmps/X86/no-gep-other-work.ll index 0cbc8dce5bd51..71c040a4a0e8e 100644 --- a/llvm/test/Transforms/MergeICmps/X86/no-gep-other-work.ll +++ b/llvm/test/Transforms/MergeICmps/X86/no-gep-other-work.ll @@ -1,5 +1,6 @@ ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py -; RUN: opt -S -passes=mergeicmps < %s | FileCheck %s +; RUN: opt -S -passes=mergeicmps < %s | FileCheck %s --check-prefixes=CHECK,GLOBAL +; RUN: opt -S -passes=mergeicmps -use-dereferenceable-at-point-semantics < %s | FileCheck %s --check-prefixes=CHECK,AT-POINT ; This does not use a GEP for the zero-offset comparison and requires a ; split for other work. @@ -9,9 +10,57 @@ target triple = "x86_64-grtev4-linux-gnu" declare void @other_work() define i1 @test(ptr dereferenceable(2) %arg, ptr dereferenceable(2) %arg1) { -; CHECK-LABEL: @test( +; GLOBAL-LABEL: @test( +; GLOBAL-NEXT: "if+entry": +; GLOBAL-NEXT: call void @other_work() +; GLOBAL-NEXT: [[MEMCMP:%.*]] = call i32 @memcmp(ptr [[ARG:%.*]], ptr [[ARG1:%.*]], i64 2) +; GLOBAL-NEXT: [[TMP0:%.*]] = icmp eq i32 [[MEMCMP]], 0 +; GLOBAL-NEXT: br label [[JOIN:%.*]] +; GLOBAL: join: +; GLOBAL-NEXT: ret i1 [[TMP0]] +; +; AT-POINT-LABEL: @test( +; AT-POINT-NEXT: entry: +; AT-POINT-NEXT: call void @other_work() +; AT-POINT-NEXT: [[ARG_OFF:%.*]] = getelementptr inbounds i8, ptr [[ARG:%.*]], i64 1 +; AT-POINT-NEXT: [[ARG1_OFF:%.*]] = getelementptr inbounds i8, ptr [[ARG1:%.*]], i64 1 +; AT-POINT-NEXT: [[ARG_OFF_VAL:%.*]] = load i8, ptr [[ARG_OFF]], align 1 +; AT-POINT-NEXT: [[ARG1_OFF_VAL:%.*]] = load i8, ptr [[ARG1_OFF]], align 1 +; AT-POINT-NEXT: [[CMP_OFF:%.*]] = icmp eq i8 [[ARG_OFF_VAL]], [[ARG1_OFF_VAL]] +; AT-POINT-NEXT: br i1 [[CMP_OFF]], label [[IF:%.*]], label [[JOIN:%.*]] +; AT-POINT: if: +; AT-POINT-NEXT: [[ARG_VAL:%.*]] = load i8, ptr [[ARG]], align 1 +; AT-POINT-NEXT: [[ARG1_VAL:%.*]] = load i8, ptr [[ARG1]], align 1 +; AT-POINT-NEXT: [[CMP:%.*]] = icmp eq i8 [[ARG_VAL]], [[ARG1_VAL]] +; AT-POINT-NEXT: br label [[JOIN]] +; AT-POINT: join: +; AT-POINT-NEXT: [[PHI:%.*]] = phi i1 [ false, [[ENTRY:%.*]] ], [ [[CMP]], [[IF]] ] +; AT-POINT-NEXT: ret i1 [[PHI]] +; +entry: + call void @other_work() + %arg.off = getelementptr inbounds i8, ptr %arg, i64 1 + %arg1.off = getelementptr inbounds i8, ptr %arg1, i64 1 + %arg.off.val = load i8, ptr %arg.off + %arg1.off.val = load i8, ptr %arg1.off + %cmp.off = icmp eq i8 %arg.off.val, %arg1.off.val + br i1 %cmp.off, label %if, label %join + +if: + %arg.val = load i8, ptr %arg + %arg1.val = load i8, ptr %arg1 + %cmp = icmp eq i8 %arg.val, %arg1.val + br label %join + +join: + %phi = phi i1 [ false, %entry ], [ %cmp, %if ] + ret i1 %phi +} + +define i1 @test_nofree(ptr dereferenceable(2) %arg, ptr dereferenceable(2) %arg1) { +; CHECK-LABEL: @test_nofree( ; CHECK-NEXT: "if+entry": -; CHECK-NEXT: call void @other_work() +; CHECK-NEXT: call void @other_work() #[[ATTR1:[0-9]+]] ; CHECK-NEXT: [[MEMCMP:%.*]] = call i32 @memcmp(ptr [[ARG:%.*]], ptr [[ARG1:%.*]], i64 2) ; CHECK-NEXT: [[TMP0:%.*]] = icmp eq i32 [[MEMCMP]], 0 ; CHECK-NEXT: br label [[JOIN:%.*]] @@ -19,7 +68,7 @@ define i1 @test(ptr dereferenceable(2) %arg, ptr dereferenceable(2) %arg1) { ; CHECK-NEXT: ret i1 [[TMP0]] ; entry: - call void @other_work() + call void @other_work() nofree %arg.off = getelementptr inbounds i8, ptr %arg, i64 1 %arg1.off = getelementptr inbounds i8, ptr %arg1, i64 1 %arg.off.val = load i8, ptr %arg.off diff --git a/llvm/test/Transforms/MergeICmps/X86/opaque-ptr.ll b/llvm/test/Transforms/MergeICmps/X86/opaque-ptr.ll index df2fa0cee08dd..f3ba65759f32a 100644 --- a/llvm/test/Transforms/MergeICmps/X86/opaque-ptr.ll +++ b/llvm/test/Transforms/MergeICmps/X86/opaque-ptr.ll @@ -1,5 +1,6 @@ ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py ; RUN: opt -S -passes=mergeicmps < %s | FileCheck %s +; RUN: opt -S -passes=mergeicmps -use-dereferenceable-at-point-semantics < %s | FileCheck %s target triple = "x86_64-unknown-unknown"