[llvm] 1b6041a - [Attributor] `willreturn` + `noreturn` = UB

[Johannes Doerfert via llvm-commits]

Author: Johannes Doerfert
Date: 2019-11-02T00:35:22-05:00
New Revision: 1b6041a9e8c537894dfda998fdd3d284b1111bd2

URL: https://github.com/llvm/llvm-project/commit/1b6041a9e8c537894dfda998fdd3d284b1111bd2
DIFF: https://github.com/llvm/llvm-project/commit/1b6041a9e8c537894dfda998fdd3d284b1111bd2.diff

LOG: [Attributor] `willreturn` + `noreturn` = UB

We gave up on `noreturn` if `willreturn` was known for a while but we
now again try to always derive `noreturn`. This is useful because a
function that is `noreturn` + `willreturn` is basically dead as
executing it would lead to undefined behavior (UB).

This came up in the IPConstantProp cases where a function only contained
a unreachable but was not marked `noreturn` which caused missed
opportunities down the line.

Added: 
    

Modified: 
    llvm/lib/Transforms/IPO/Attributor.cpp
    llvm/test/Transforms/FunctionAttrs/fn_noreturn.ll
    llvm/test/Transforms/FunctionAttrs/internal-noalias.ll
    llvm/test/Transforms/FunctionAttrs/norecurse.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index 4c8bb0f51e96..ee7166e381ae 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -3076,7 +3076,7 @@ struct AANoReturnImpl : public AANoReturn {
   void initialize(Attributor &A) override {
     AANoReturn::initialize(A);
     Function *F = getAssociatedFunction();
-    if (!F || F->hasFnAttribute(Attribute::WillReturn))
+    if (!F)
       indicatePessimisticFixpoint();
   }
 
@@ -3087,9 +3087,6 @@ struct AANoReturnImpl : public AANoReturn {
 
   /// See AbstractAttribute::updateImpl(Attributor &A).
   virtual ChangeStatus updateImpl(Attributor &A) override {
-    const auto &WillReturnAA = A.getAAFor<AAWillReturn>(*this, getIRPosition());
-    if (WillReturnAA.isKnownWillReturn())
-      return indicatePessimisticFixpoint();
     auto CheckForNoReturn = [](Instruction &) { return false; };
     if (!A.checkForAllInstructions(CheckForNoReturn, *this,
                                    {(unsigned)Instruction::Ret}))

diff  --git a/llvm/test/Transforms/FunctionAttrs/fn_noreturn.ll b/llvm/test/Transforms/FunctionAttrs/fn_noreturn.ll
index d0ccb87485be..2b15e0780df7 100644
--- a/llvm/test/Transforms/FunctionAttrs/fn_noreturn.ll
+++ b/llvm/test/Transforms/FunctionAttrs/fn_noreturn.ll
@@ -125,10 +125,12 @@ cond.end:                                         ; preds = %cond.false, %cond.t
 }
 
 
-; TEST 6: willreturn means *not* no-return
-; CHECK:      Function Attrs: nofree norecurse nosync nounwind readnone willreturn
+; TEST 6a: willreturn means *not* no-return or UB
+; FIXME: we should derive "UB" as an argument and report it to the user on request.
+
+; CHECK:      Function Attrs: nofree norecurse noreturn nosync nounwind readnone willreturn
 ; CHECK-NEXT: define i32 @endless_loop_but_willreturn
-define i32 @endless_loop_but_willreturn(i32 %a) willreturn {
+define i32 @endless_loop_but_willreturn() willreturn {
 entry:
   br label %while.body
 
@@ -136,4 +138,12 @@ while.body:                                       ; preds = %entry, %while.body
   br label %while.body
 }
 
+; TEST 6b: willreturn means *not* no-return or UB
+; CHECK:      Function Attrs: nofree norecurse noreturn nosync nounwind readnone willreturn
+; CHECK-NEXT: define i32 @UB_and_willreturn
+define i32 @UB_and_willreturn() willreturn {
+entry:
+  unreachable
+}
+
 attributes #0 = { noinline nounwind uwtable }

diff  --git a/llvm/test/Transforms/FunctionAttrs/internal-noalias.ll b/llvm/test/Transforms/FunctionAttrs/internal-noalias.ll
index 2ccb4b67e564..9afea1fbf8d0 100644
--- a/llvm/test/Transforms/FunctionAttrs/internal-noalias.ll
+++ b/llvm/test/Transforms/FunctionAttrs/internal-noalias.ll
@@ -1,4 +1,4 @@
-; RUN: opt -S -passes=attributor -aa-pipeline='basic-aa' -attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=2 < %s | FileCheck %s
+; RUN: opt -S -passes=attributor -aa-pipeline='basic-aa' -attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=7 < %s | FileCheck %s
 
 define dso_local i32 @visible(i32* noalias %A, i32* noalias %B) #0 {
 entry:

diff  --git a/llvm/test/Transforms/FunctionAttrs/norecurse.ll b/llvm/test/Transforms/FunctionAttrs/norecurse.ll
index 0e67a600f344..132396fc37fb 100644
--- a/llvm/test/Transforms/FunctionAttrs/norecurse.ll
+++ b/llvm/test/Transforms/FunctionAttrs/norecurse.ll
@@ -1,6 +1,6 @@
 ; RUN: opt < %s -basicaa -functionattrs -rpo-functionattrs -S | FileCheck %s --check-prefixes=CHECK,BOTH
 ; RUN: opt < %s -aa-pipeline=basic-aa -passes='cgscc(function-attrs),rpo-functionattrs' -S | FileCheck %s --check-prefixes=CHECK,BOTH
-; RUN: opt -passes=attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=4 -S < %s | FileCheck %s --check-prefixes=ATTRIBUTOR,BOTH
+; RUN: opt -passes=attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=2 -S < %s | FileCheck %s --check-prefixes=ATTRIBUTOR,BOTH
 
 ; CHECK: Function Attrs
 ; CHECK-SAME: norecurse nounwind readnone