[llvm] 3314685 - [IR] Consider non-willreturn as side effect (PR50511)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 26 07:35:27 PDT 2021
Author: Nikita Popov
Date: 2021-07-26T16:35:14+02:00
New Revision: 33146857e9840a92840d48bbc3483e34ea545fc7
URL: https://github.com/llvm/llvm-project/commit/33146857e9840a92840d48bbc3483e34ea545fc7
DIFF: https://github.com/llvm/llvm-project/commit/33146857e9840a92840d48bbc3483e34ea545fc7.diff
LOG: [IR] Consider non-willreturn as side effect (PR50511)
This adjusts mayHaveSideEffect() to return true for !willReturn()
instructions. Just like other side-effects, non-willreturn calls
(aka "divergence") cannot be removed and cannot be reordered relative
to other side effects. This fixes a number of bugs where
non-willreturn calls are either incorrectly dropped or moved. In
particular, it also fixes the last open problem in
https://bugs.llvm.org/show_bug.cgi?id=50511.
I performed a cursory review of all current mayHaveSideEffect()
uses, which convinced me that these are indeed the desired default
semantics. Places that do not want to consider non-willreturn as a
sideeffect generally do not want mayHaveSideEffect() semantics at
all. I identified two such cases, which are addressed by D106591
and D106742. Finally, there is a use in SCEV for which we don't
really have an appropriate API right now -- what it wants is
basically "would this be considered forward progress". I've just
spelled out the previous semantics there.
Differential Revision: https://reviews.llvm.org/D106749
Added:
Modified:
llvm/include/llvm/IR/Instruction.h
llvm/lib/Analysis/DemandedBits.cpp
llvm/lib/Analysis/ScalarEvolution.cpp
llvm/lib/IR/Instruction.cpp
llvm/lib/Transforms/Scalar/ADCE.cpp
llvm/test/Transforms/LICM/sinking.ll
llvm/test/Transforms/LoopDeletion/noop-loops-with-subloops.ll
llvm/test/Transforms/SCCP/calltest.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index 391fb32c9ca4b..5ca9c69268eae 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -627,11 +627,16 @@ class Instruction : public User,
/// Return true if the instruction may have side effects.
///
+ /// Side effects are:
+ /// * Writing to memory.
+ /// * Unwinding.
+ /// * Not returning (e.g. an infinite loop).
+ ///
/// Note that this does not consider malloc and alloca to have side
/// effects because the newly allocated memory is completely invisible to
/// instructions which don't use the returned value. For cases where this
/// matters, isSafeToSpeculativelyExecute may be more appropriate.
- bool mayHaveSideEffects() const { return mayWriteToMemory() || mayThrow(); }
+ bool mayHaveSideEffects() const;
/// Return true if the instruction can be removed if the result is unused.
///
diff --git a/llvm/lib/Analysis/DemandedBits.cpp b/llvm/lib/Analysis/DemandedBits.cpp
index 467dea9c02366..ca6d58fac825b 100644
--- a/llvm/lib/Analysis/DemandedBits.cpp
+++ b/llvm/lib/Analysis/DemandedBits.cpp
@@ -80,7 +80,7 @@ void DemandedBitsWrapperPass::print(raw_ostream &OS, const Module *M) const {
static bool isAlwaysLive(Instruction *I) {
return I->isTerminator() || isa<DbgInfoIntrinsic>(I) || I->isEHPad() ||
- I->mayHaveSideEffects() || !I->willReturn();
+ I->mayHaveSideEffects();
}
void DemandedBits::determineLiveOperandBits(
diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index 3a1182ca01e00..df38a850c287d 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -6677,7 +6677,7 @@ ScalarEvolution::getLoopProperties(const Loop *L) {
if (auto *SI = dyn_cast<StoreInst>(I))
return !SI->isSimple();
- return I->mayHaveSideEffects();
+ return I->mayThrow() || I->mayWriteToMemory();
};
LoopProperties LP = {/* HasNoAbnormalExits */ true,
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index a6fba2a7a2402..649a85e6a4c2e 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -663,6 +663,10 @@ bool Instruction::mayThrow() const {
return isa<ResumeInst>(this);
}
+bool Instruction::mayHaveSideEffects() const {
+ return mayWriteToMemory() || mayThrow() || !willReturn();
+}
+
bool Instruction::isSafeToRemove() const {
return (!isa<CallInst>(this) || !this->mayHaveSideEffects()) &&
!this->isTerminator();
diff --git a/llvm/lib/Transforms/Scalar/ADCE.cpp b/llvm/lib/Transforms/Scalar/ADCE.cpp
index 12a3e678393a6..6f3fdb88eda50 100644
--- a/llvm/lib/Transforms/Scalar/ADCE.cpp
+++ b/llvm/lib/Transforms/Scalar/ADCE.cpp
@@ -326,7 +326,7 @@ void AggressiveDeadCodeElimination::initialize() {
bool AggressiveDeadCodeElimination::isAlwaysLive(Instruction &I) {
// TODO -- use llvm::isInstructionTriviallyDead
- if (I.isEHPad() || I.mayHaveSideEffects() || !I.willReturn()) {
+ if (I.isEHPad() || I.mayHaveSideEffects()) {
// Skip any value profile instrumentation calls if they are
// instrumenting constants.
if (isInstrumentsConstant(I))
diff --git a/llvm/test/Transforms/LICM/sinking.ll b/llvm/test/Transforms/LICM/sinking.ll
index ee7f6338e6ad6..e8660695aa79f 100644
--- a/llvm/test/Transforms/LICM/sinking.ll
+++ b/llvm/test/Transforms/LICM/sinking.ll
@@ -979,16 +979,16 @@ try.cont:
ret void
}
-; TODO: Should not get sunk.
define i32 @not_willreturn(i8* %p) {
; CHECK-LABEL: @not_willreturn(
+; CHECK-NEXT: [[X:%.*]] = call i32 @getv() #[[ATTR5:[0-9]+]]
; CHECK-NEXT: br label [[LOOP:%.*]]
; CHECK: loop:
; CHECK-NEXT: store volatile i8 0, i8* [[P:%.*]], align 1
; CHECK-NEXT: br i1 true, label [[LOOP]], label [[OUT:%.*]]
; CHECK: out:
-; CHECK-NEXT: [[X_LE:%.*]] = call i32 @getv() #[[ATTR5:[0-9]+]]
-; CHECK-NEXT: ret i32 [[X_LE]]
+; CHECK-NEXT: [[X_LCSSA:%.*]] = phi i32 [ [[X]], [[LOOP]] ]
+; CHECK-NEXT: ret i32 [[X_LCSSA]]
;
br label %loop
diff --git a/llvm/test/Transforms/LoopDeletion/noop-loops-with-subloops.ll b/llvm/test/Transforms/LoopDeletion/noop-loops-with-subloops.ll
index 9163c9cd401e7..bf5288085df14 100644
--- a/llvm/test/Transforms/LoopDeletion/noop-loops-with-subloops.ll
+++ b/llvm/test/Transforms/LoopDeletion/noop-loops-with-subloops.ll
@@ -395,11 +395,16 @@ exit:
}
; Inner infinite loop hidden behind a call.
-; TODO: Loop should not get deleted.
define void @not_willreturn() {
; CHECK-LABEL: @not_willreturn(
; CHECK-NEXT: entry:
-; CHECK-NEXT: br label [[EXIT:%.*]]
+; CHECK-NEXT: br label [[LOOP:%.*]]
+; CHECK: loop:
+; CHECK-NEXT: [[IV:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[LOOP]] ]
+; CHECK-NEXT: call void @sideeffect() #[[ATTR2:[0-9]+]]
+; CHECK-NEXT: [[IV_NEXT]] = add nuw i32 [[IV]], 1
+; CHECK-NEXT: [[C:%.*]] = icmp ult i32 [[IV]], 100
+; CHECK-NEXT: br i1 [[C]], label [[LOOP]], label [[EXIT:%.*]]
; CHECK: exit:
; CHECK-NEXT: ret void
;
diff --git a/llvm/test/Transforms/SCCP/calltest.ll b/llvm/test/Transforms/SCCP/calltest.ll
index de793635600af..2a6445fcb6810 100644
--- a/llvm/test/Transforms/SCCP/calltest.ll
+++ b/llvm/test/Transforms/SCCP/calltest.ll
@@ -34,9 +34,9 @@ define i32 @test_1() {
ret i32 0
}
-; TODO: Call should not get dropped.
define i32 @test_not_willreturn() {
; CHECK-LABEL: @test_not_willreturn(
+; CHECK-NEXT: [[TMP1:%.*]] = call [[EMPTY:%.*]] @has_side_effects() #[[ATTR1:[0-9]+]]
; CHECK-NEXT: ret i32 0
;
%1 = call %empty @has_side_effects() nounwind readonly
More information about the llvm-commits
mailing list