[llvm] c3b06d0 - [InstCombine] keep assumption before sinking calls
via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 30 16:16:42 PDT 2019
Author: tyker
Date: 2019-10-31T00:15:19+01:00
New Revision: c3b06d0c393e533eab712922911d14e5a079fa5d
URL: https://github.com/llvm/llvm-project/commit/c3b06d0c393e533eab712922911d14e5a079fa5d
DIFF: https://github.com/llvm/llvm-project/commit/c3b06d0c393e533eab712922911d14e5a079fa5d.diff
LOG: [InstCombine] keep assumption before sinking calls
Summary:
in the following C code the branch is not removed by clang in O3.
```
int f1(char* p) {
int i1 = __builtin_strlen(p);
if (!p)
return -1;
return i1;
}
```
The issue is that the call to strlen is sunk to the following block by instcombine. In its new place the call to strlen doesn't dominate the use in the icmp anymore so value tracking can't see that p cannot be null.
This patch resolves the issue by inserting an assumption at the place of the call before sinking a call when that call can be used to prove an argument to be nonnull.
This resolves this issue at O3.
Reviewers: majnemer, xbolva00, fhahn, jdoerfert, spatel, efriedma
Reviewed By: jdoerfert
Subscribers: hiraditya, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D69477
Added:
llvm/test/Transforms/InstCombine/assume-replacing-call.ll
Modified:
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index ecb486c544e0..c4f9be0a15de 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3115,7 +3115,8 @@ Instruction *InstCombiner::visitLandingPadInst(LandingPadInst &LI) {
/// beginning of DestBlock, which can only happen if it's safe to move the
/// instruction past all of the instructions between it and the end of its
/// block.
-static bool TryToSinkInstruction(Instruction *I, BasicBlock *DestBlock) {
+static bool TryToSinkInstruction(Instruction *I, BasicBlock *DestBlock,
+ InstCombiner::BuilderTy &Builder) {
assert(I->hasOneUse() && "Invariants didn't hold!");
BasicBlock *SrcBlock = I->getParent();
@@ -3149,6 +3150,24 @@ static bool TryToSinkInstruction(Instruction *I, BasicBlock *DestBlock) {
if (Scan->mayWriteToMemory())
return false;
}
+
+ // If this instruction was providing nonnull guarantees insert assumptions for
+ // nonnull call site arguments.
+ if (auto CS = dyn_cast<CallBase>(I)) {
+ for (unsigned Idx = 0; Idx != CS->getNumArgOperands(); Idx++)
+ if (CS->paramHasAttr(Idx, Attribute::NonNull) ||
+ (CS->paramHasAttr(Idx, Attribute::Dereferenceable) &&
+ !llvm::NullPointerIsDefined(CS->getFunction(),
+ CS->getArgOperand(Idx)
+ ->getType()
+ ->getPointerAddressSpace()))) {
+ Value *V = CS->getArgOperand(Idx);
+
+ Builder.SetInsertPoint(I->getParent(), I->getIterator());
+ Builder.CreateAssumption(Builder.CreateIsNotNull(V));
+ }
+ }
+
BasicBlock::iterator InsertPos = DestBlock->getFirstInsertionPt();
I->moveBefore(&*InsertPos);
++NumSunkInst;
@@ -3283,7 +3302,7 @@ bool InstCombiner::run() {
// otherwise), we can keep going.
if (UserIsSuccessor && UserParent->getUniquePredecessor()) {
// Okay, the CFG is simple enough, try to sink this instruction.
- if (TryToSinkInstruction(I, UserParent)) {
+ if (TryToSinkInstruction(I, UserParent, Builder)) {
LLVM_DEBUG(dbgs() << "IC: Sink: " << *I << '\n');
MadeIRChange = true;
// We'll add uses of the sunk instruction below, but since sinking
diff --git a/llvm/test/Transforms/InstCombine/assume-replacing-call.ll b/llvm/test/Transforms/InstCombine/assume-replacing-call.ll
new file mode 100644
index 000000000000..e039f61f67e8
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/assume-replacing-call.ll
@@ -0,0 +1,192 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -instcombine -S | FileCheck %s
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @test_sink_replacement1(i8* %p) {
+; CHECK-LABEL: @test_sink_replacement1(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[TMP0:%.*]] = icmp ne i8* [[P:%.*]], null
+; CHECK-NEXT: call void @llvm.assume(i1 [[TMP0]])
+; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i8* [[P]], null
+; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+; CHECK: if.then:
+; CHECK-NEXT: br label [[CLEANUP:%.*]]
+; CHECK: if.end:
+; CHECK-NEXT: [[CALL:%.*]] = call i64 @func_test(i8* nonnull [[P]]) #2
+; CHECK-NEXT: [[CONV:%.*]] = trunc i64 [[CALL]] to i32
+; CHECK-NEXT: br label [[CLEANUP]]
+; CHECK: cleanup:
+; CHECK-NEXT: [[RETVAL_0:%.*]] = phi i32 [ [[CONV]], [[IF_END]] ], [ -1, [[IF_THEN]] ]
+; CHECK-NEXT: ret i32 [[RETVAL_0]]
+;
+entry:
+ %call = call i64 @func_test(i8* nonnull %p) #2
+ %conv = trunc i64 %call to i32
+ %tobool = icmp ne i8* %p, null
+ br i1 %tobool, label %if.end, label %if.then
+
+if.then:
+ br label %cleanup
+
+if.end:
+ br label %cleanup
+
+cleanup:
+ %retval.0 = phi i32 [ %conv, %if.end ], [ -1, %if.then ]
+ ret i32 %retval.0
+}
+
+define i32 @test_sink_replacement2(i8* %p) {
+; CHECK-LABEL: @test_sink_replacement2(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[TMP0:%.*]] = icmp ne i8* [[P:%.*]], null
+; CHECK-NEXT: call void @llvm.assume(i1 [[TMP0]])
+; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i8* [[P]], null
+; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+; CHECK: if.then:
+; CHECK-NEXT: br label [[CLEANUP:%.*]]
+; CHECK: if.end:
+; CHECK-NEXT: [[CALL:%.*]] = call i64 @func_test(i8* nonnull dereferenceable(1) [[P]]) #2
+; CHECK-NEXT: [[CONV:%.*]] = trunc i64 [[CALL]] to i32
+; CHECK-NEXT: br label [[CLEANUP]]
+; CHECK: cleanup:
+; CHECK-NEXT: [[RETVAL_0:%.*]] = phi i32 [ [[CONV]], [[IF_END]] ], [ -1, [[IF_THEN]] ]
+; CHECK-NEXT: ret i32 [[RETVAL_0]]
+;
+entry:
+ %call = call i64 @func_test(i8* dereferenceable(1) %p) #2
+ %conv = trunc i64 %call to i32
+ %tobool = icmp ne i8* %p, null
+ br i1 %tobool, label %if.end, label %if.then
+
+if.then:
+ br label %cleanup
+
+if.end:
+ br label %cleanup
+
+cleanup:
+ %retval.0 = phi i32 [ %conv, %if.end ], [ -1, %if.then ]
+ ret i32 %retval.0
+}
+
+define i32 @test_sink_replacement3(i8* %p) {
+; CHECK-LABEL: @test_sink_replacement3(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[TMP0:%.*]] = icmp ne i8* [[P:%.*]], null
+; CHECK-NEXT: call void @llvm.assume(i1 [[TMP0]])
+; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i8* [[P]], null
+; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+; CHECK: if.then:
+; CHECK-NEXT: br label [[CLEANUP:%.*]]
+; CHECK: if.end:
+; CHECK-NEXT: [[CALL:%.*]] = call i64 @func_test_nonnull(i8* [[P]]) #2
+; CHECK-NEXT: [[CONV:%.*]] = trunc i64 [[CALL]] to i32
+; CHECK-NEXT: br label [[CLEANUP]]
+; CHECK: cleanup:
+; CHECK-NEXT: [[RETVAL_0:%.*]] = phi i32 [ [[CONV]], [[IF_END]] ], [ -1, [[IF_THEN]] ]
+; CHECK-NEXT: ret i32 [[RETVAL_0]]
+;
+entry:
+ %call = call i64 @func_test_nonnull(i8* %p) #2
+ %conv = trunc i64 %call to i32
+ %tobool = icmp ne i8* %p, null
+ br i1 %tobool, label %if.end, label %if.then
+
+if.then:
+ br label %cleanup
+
+if.end:
+ br label %cleanup
+
+cleanup:
+ %retval.0 = phi i32 [ %conv, %if.end ], [ -1, %if.then ]
+ ret i32 %retval.0
+}
+
+define i32 @test_sink_replacement4(i8* %p) {
+; CHECK-LABEL: @test_sink_replacement4(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br label [[A:%.*]]
+; CHECK: a:
+; CHECK-NEXT: [[TMP0:%.*]] = icmp ne i8* [[P:%.*]], null
+; CHECK-NEXT: call void @llvm.assume(i1 [[TMP0]])
+; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i8* [[P]], null
+; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+; CHECK: if.then:
+; CHECK-NEXT: br label [[CLEANUP:%.*]]
+; CHECK: if.end:
+; CHECK-NEXT: [[CALL:%.*]] = call i64 @func_test(i8* nonnull dereferenceable(1) [[P]]) #2
+; CHECK-NEXT: [[CONV:%.*]] = trunc i64 [[CALL]] to i32
+; CHECK-NEXT: br label [[CLEANUP]]
+; CHECK: cleanup:
+; CHECK-NEXT: [[RETVAL_0:%.*]] = phi i32 [ [[CONV]], [[IF_END]] ], [ -1, [[IF_THEN]] ]
+; CHECK-NEXT: ret i32 [[RETVAL_0]]
+;
+entry:
+ br label %a
+a:
+ %call = call i64 @func_test(i8* dereferenceable(1) %p) #2
+ %conv = trunc i64 %call to i32
+ %tobool = icmp ne i8* %p, null
+ br i1 %tobool, label %if.end, label %if.then
+
+if.then:
+ br label %cleanup
+
+if.end:
+ br label %cleanup
+
+cleanup:
+ %retval.0 = phi i32 [ %conv, %if.end ], [ -1, %if.then ]
+ ret i32 %retval.0
+}
+
+define i32 @test_sink_replacement5(i64 %i) {
+; CHECK-LABEL: @test_sink_replacement5(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[P:%.*]] = inttoptr i64 [[I:%.*]] to i8*
+; CHECK-NEXT: br label [[A:%.*]]
+; CHECK: a:
+; CHECK-NEXT: [[TMP0:%.*]] = icmp ne i64 [[I]], 0
+; CHECK-NEXT: call void @llvm.assume(i1 [[TMP0]])
+; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i64 [[I]], 0
+; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+; CHECK: if.then:
+; CHECK-NEXT: br label [[CLEANUP:%.*]]
+; CHECK: if.end:
+; CHECK-NEXT: [[CALL:%.*]] = call i64 @func_test(i8* nonnull dereferenceable(1) [[P]]) #2
+; CHECK-NEXT: [[CONV:%.*]] = trunc i64 [[CALL]] to i32
+; CHECK-NEXT: br label [[CLEANUP]]
+; CHECK: cleanup:
+; CHECK-NEXT: [[RETVAL_0:%.*]] = phi i32 [ [[CONV]], [[IF_END]] ], [ -1, [[IF_THEN]] ]
+; CHECK-NEXT: ret i32 [[RETVAL_0]]
+;
+entry:
+ %p = inttoptr i64 %i to i8*
+ br label %a
+a:
+ %call = call i64 @func_test(i8* dereferenceable(1) %p) #2
+ %conv = trunc i64 %call to i32
+ %tobool = icmp ne i8* %p, null
+ br i1 %tobool, label %if.end, label %if.then
+
+if.then:
+ br label %cleanup
+
+if.end:
+ br label %cleanup
+
+cleanup:
+ %retval.0 = phi i32 [ %conv, %if.end ], [ -1, %if.then ]
+ ret i32 %retval.0
+}
+
+declare i64 @func_test(i8* %0) #1
+
+declare i64 @func_test_nonnull(i8* nonnull %0) #3
+
+attributes #1 = { readonly }
+attributes #2 = { nounwind }
+attributes #3 = { readonly }
More information about the llvm-commits
mailing list