[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