[PATCH] D93946: [FuncAttrs] Infer noreturn

[Madhur Amilkanthwar via Phabricator via llvm-commits]

madhur13490 added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/FunctionAttrs.cpp:1400
+
+    bool CanReturn = false;
+    // The function can return if any basic blocks terminating with a ReturnInst
----------------
aeubanks wrote:
> madhur13490 wrote:
> > I think you can actually avoid this variable. You can declare "FoundNoReturnCall" outside the loops and define it in each iteration of the outer loop. If (!FoundNoReturnCall) in outer loop then break and if (FoundNoReturnCall) outside the loop then F->setDoesNotReturn(). CanReturn is acting as an accumulator here but if you declare FoundNoReturnCall outside the loop then it can act for the same purpose.
> I'm not following, could you post the code?
> This is basically an `all(any())` where the `all` means "every basic block must" and the `any` means "any instruction in the block doesn't return". I believe you need two variables for `all(any())`.
Ah! I misremembered `noreturn` as `any(any)`.  But I guess you can still do it with one variable if you have `unsetDoesNotReturn` in llvm::Function. Something like below,

```
 bool Changed = false;
    F->setDoesNotReturn();
    for (BasicBlock &BB : *F) {
      if (!isa<ReturnInst>(BB.getTerminator()))
        continue;
      bool BasicBlockCanReachReturn = true;
      for (Instruction &I : BB) {
        if (instructionDoesNotReturn(I)) {
          BasicBlockCanReachReturn = false;
          break;
        }
      }
      if (BasicBlockCanReachReturn) {
        F->unsetDoesNotReturn();
        Changed = true;
        break;
      }
    }

```
The method is not currently present but I see no reason for ignoring it. I see removeFnAttr being use to one can add such method. More generally, there can be methods like changeNoReturnAttrib(bool) which changes the state to the input bool; if changeNoReturnAttrib(true) then it adds the attribute, if changeNoReturnAttrib(false) it remove the attribute - of course after checking redundancy.

However,  I think we can continue for now.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93946