[PATCH] D59978: [Attributor] Deduce the "no-return" attribute for functions

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: llvm/test/Transforms/FunctionAttrs/arg_nocapture.ll:2
 ; RUN: opt -functionattrs -attributor -attributor-disable=false -S < %s | FileCheck %s
-; RUN: opt -functionattrs -attributor -attributor-disable=false -attributor-verify=true -S < %s | FileCheck %s
 ;
----------------
jdoerfert wrote:
> Meinersbur wrote:
> > Unrelated change?
> no.
> 
> "Verification" is tricky right now, we haven't found a good system yet. This change will break "verification" but not because it actually does something wrong but because the "verification" is not smart enough. As I said, we do not have a good way of replacing the mechanism yet so I disable it here for now.
Maybe add a TODO/FIXME remark about it?


================
Comment at: llvm/test/Transforms/FunctionAttrs/fn_noreturn.ll:37
 entry:
   %call = call i32 @srec16(i32 %a)
   %call1 = call i32 @srec16(i32 %call)
----------------
jdoerfert wrote:
> Meinersbur wrote:
> > Out of curiosity, where/how does the Attributor determine this is an infinite recursion? Wouldn't a single recursive call be enough?
> A single call is sufficient, when I wrote the test I had something else in mind though.
> No-return is now deduced here because:
> - Initially we assume `%call` is `noreturn`,
> - which causes `%call1` and the rest to be dead (`AAIsDead`),
> - which causes the return to be never encountered by `AAReturnedValues` (after D65243),
> - which causes `%call` to stay assumed `noreturn`.
> So a single call suffices.
> 
> The cascade might be a nice test to make sure we do not go down the rabbit hole.
On windows, you can catch a stackoverflow exception. E.g. in this program:
```
#include <cstdlib>
#include <cstdio>

void overflow() {
  overflow();
}

int catchoverflow() {
  __try  {
    overflow();
    printf("Exception NOT caught\n");
    return 0;
  } 
  __except(true) {
    printf("Exception caught\n");
    return 1;
  }
  return 2;
}

int main() {
  auto result = catchoverflow();
  printf("Done execution result=%i\n", result);
  return EXIT_SUCCESS;
}
```
results in (compiled with either msvc or clang-cl):
```
Exception caught
Done execution result=1
```
The LLVM-IR for this contains:
```
entry:
  %retval = alloca i32, align 4
  %__exception_code = alloca i32, align 4
  invoke void @"?overflow@@YAXXZ"() #6
          to label %invoke.cont unwind label %catch.dispatch

catch.dispatch:                                   ; preds = %invoke.cont, %entry
  %0 = catchswitch within none [label %__except] unwind to caller
```
(entire file: {F9712200})

Would `catchoverflow()` still get a `noreturn` attribute?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59978