[PATCH] D64162: Summary: [Attributor] Liveness analysis abstract attribute used to indicate which BasicBlocks are dead and can therefore be ignored.

[Stefan Stipanovic via Phabricator via llvm-commits]

sstefan1 marked 2 inline comments as done.
sstefan1 added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:1103
+
+    I = I->getNextNode();
+  }
----------------
jdoerfert wrote:
> Let's assume you get in here for an assumed noreturn call `NC` which is not assume noreturn anymore, you iterate over the block until you find one that is noreturn. In that case you return `false` above which would *not* remove the former assumed noreturn call `NC` from the `NoReturnCalls` set, causing us to introduce an unreachable at the end! You have to return `true` whenever the original instruction `I` is not assumed noreturn anymore, e.g., one you call `I->getNextNode` we cannot return `false` anymore
> 
> 
> test case sketch
> 
> ```
> bb:
>   call %assumed_noreturn_but_actually_return
>   call %noreturn
> ```
Ok, I was wrong about this. Now it will return false only if we are looking at a noreturn call that is already in `NoReturnCalls`, in other words, still noreturn.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64162/new/

https://reviews.llvm.org/D64162