[PATCH] D83906: [CodeGen] Emit a call instruction instead of an invoke if the called llvm function is marked nounwind

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D83906#4182777 <https://reviews.llvm.org/D83906#4182777>, @rjmccall wrote:

> In D83906#4182287 <https://reviews.llvm.org/D83906#4182287>, @dexonsmith wrote:
>
>> - At IRGen time, you know the LLVM attributes have not been adjusted after the optimized refined the function's behaviour. It should be safe to have IPA peepholes, as long as IRGen's other peepholes don't refine behaviour and add attributes based on that.
>> - In the optimizer, if you're looking at de-refineable function then you don't know which attributes come directly from the source and which were implied by optimizer refinements. You can't trust you'll get the same function attributes at runtime.
>
> Hmm.  I see what you're saying, but it's an interesting question how it applies here.  In principle, the optimizer should not be changing the observable semantics of functions, which certainly includes things like whether the function throws.  Maybe the optimizer can only figure out that a function throws in one TU, but if it "figures that out" and then a function with supposedly the same semantics actually does throw — not just retains the static ability to throw on a path that happens not to be taken dynamically, but actually throws at runtime — then arguably something has gone badly wrong.

I believe in my example, it's kind of the reverse. Only one TU *remembers* that the function can throw; the other one "forgets" because it has optimized its variant not to `throw`.

Maybe it's useful to note that, while `maybe_nounwind` has no UB, whether it throws or not depends on thread timing, and is generally non-reproducible (run it twice, you can get different results). In the TU that forgets, the optimizer is choosing to assume that the two adjacent atomic loads happen so quickly that no store happens in between; choosing the thread timing where there's no store to contend with. This is a valid refinement of the original source semantics -- optimizers are allowed to CSE adjacent atomic loads.

> As I recall, the de-refinement discussion was originally about properties that are *not* invariant to optimization in this way, things like whether the function uses one of its arguments.  Those properties are not generally considered to be part of the function's externally-observable semantics.

The example described in the referenced de-refinement commit is where a function that writes to memory is refined to `readnone`. I think my `nounwind` example above is analogous.

Here's the original from https://reviews.llvm.org/D18634:

> For instance, FunctionAttrs cannot assume a comdat function is
> actually readnone even if it does not have any loads or stores in
> it; since there may have been loads and stores in the "original
> function" that were refined out in the currently visible variant, and
> at the link step the linker may in fact choose an implementation with
> a load or a store. As an example, consider a function that does two
> atomic loads from the same memory location, and writes to memory only
> if the two values are not equal. The optimizer is allowed to refine
> this function by first CSE'ing the two loads, and the folding the
> comparision to always report that the two values are equal. Such a
> refined variant will look like it is readonly. However, the
> unoptimized version of the function can still write to memory (since
> the two loads can result in different values), and selecting the
> unoptimized version at link time will retroactively invalidate
> transforms we may have done under the assumption that the function
> does not write to memory.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83906