[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#4181981 <https://reviews.llvm.org/D83906#4181981>, @hoy wrote:

> That said, the LLVM optimizer does not strictly subsume the front-end because of how it fails to handle `linkonce_odr` functions as in https://reviews.llvm.org/D18634. I'm wondering how common the `linkonce_odr` linkage is for C++. In @wlei's example, none of the functions there is `linkonce_odr`. Is there a particular source-level annotate that specifies functions to be `linkonce_odr`?

In C++, you get `linkonce_odr` all over the place. It's basically all functions that are defined in C++ headers that are available for inlining.

- any function marked `inline`
- any function in a class/struct whose declaration is its definition (approximately all templated code)

A few exceptions:

- If a function is explicitly instantiated (e.g., member functions of `T<int>` if `template class T<int>;`), it gets `weak_odr`, which IIRC cannot be de-refined?
- If a function has local linkage (like free functions with `static inline`), it gets `internal`, which cannot be de-refined.
- If a function is marked `inline` inside an `extern "C"` block, it gets `available_externally`. This can also be de-refined (but without ODR, you wouldn't be tempted to optimize based on its attributes anyway).



> It sounds to me that at link time only equivalent symbols can replace each other. Then de-refining some of those equivalent symbols should not affect their semantics as far as nothrow is concerned? Just as @rjmccall pointed out, the C++ language guarantee we're starting from is that the source semantics of all versions of the function are identical.

The rule is subtly different. Only symbols that are source-equivalent can replace each other. But they aren't necessarily equivalent to the function you see, which may have been refined by optimization.

Here's a concrete example. Say we have function `maybe_nounwind` that is not `nounwind` at the source level, and a `catch_all` function that wraps it.

  // Defined in header.
  extern std::atomic<int> Global;
  
  // LLVM: linkonce_odr
  inline int maybe_nounwind(int In) {
    int Read1 = Global;
    int Read2 = Global;
    if (Read1 != Read2)
      throw 0;
  
    return /* Big, non-inlineable computation on In */;
  }
  
  // Defined in source.
  // LLVM: nounwind
  int catch_all(int In) {
    try {
      return maybe_nounwind(In);
    } catch (...) {
      return -1;
    }
  }

There's no UB here, since comparing two atomic loads is allowed. In rare cases, an unoptimized `maybe_nounwind` could throw, if another thread is changing the value of `Global` between the two loads.

But the optimizer will probably CSE the two atomic loads since it's allowed to assume that both loads happen at the same time. This refines `maybe_nounwind`. It'll turn into IR equivalent to:

  // Defined in header. Then optimized.
  // LLVM: linkonce_odr nounwind readnone
  inline int maybe_nounwind(int In) {
    return /* Big, non-inlineable computation on In */;
  }
  
  // Defined in source. Then optimized.
  // LLVM: nounwind
  int catch_all(int In) {
    try {
      return maybe_nounwind(In);
    } catch (...) {
      return -1;
    }
  }

It's important that `catch_all` is NOT optimized based on `maybe_nounwind`'s new `nounwind` attribute. At link time, it's possible for the linker to choose an unoptimized copy of `maybe_nounwind`. Just in case it does, `catch_all` needs to keep its `try/catch` block, since unoptimized `maybe_nounwind` can throw. Similarly, `catch_all` should not be marked `readnone`, even though the refined/optimized `maybe_nounwind` is `readnone`, since a de-refined copy reads from memory.

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

> There are *some* properties we can still assume about `linkonce_odr` functions despite them being replaceable at link time.  The high-level language guarantee we're starting from is that the source semantics of all versions of the function are identical.  The version of the function we're looking at has been transformed from the original source — it is, after all, now LLVM IR, not C/C++ — but it has presumably faithfully preserved the source semantics.  We can therefore rely on any properties of the semantics that are required to be preserved by transformation, which includes things like "does it terminate", "what value does it return", "what side effects does it perform", and so on.  What we can't rely on are properties of the implementation that are not required to be preserved by transformation, like whether or not it uses a certain argument — transformations are permitted to change that.



- 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83906