[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 cfe-commits at lists.llvm.org
Thu Mar 9 14:37:08 PST 2023


dexonsmith added a subscriber: sanjoy.
dexonsmith added a comment.

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

> So your argument is that it would not be possible to recognize that we're doing such an optimization and mark the function as having had a possible semantics change?

I suspect it would be error-prone to do that precisely. I'd bet there are a variety of hard-to-reason about examples. Originally, when @sanjoy was first describing this problem (to me and others), his examples all had UB in the original code (e.g., reading and writing to globals in different threads). Eventually he invented the adjacent-atomic-load device, described above, which does not rely on UB in the original code. I just assume there are more devices out there that we don't know about or understand.

Maybe it would be useful to do it imprecisely? E.g, have all transformation passes mark all functions as changed (or drop a `pristine` attribute)? Then at least you know whether it has come directly from IRGen.

Not sure if it's valuable enough though. I don't think the regressions were as bad as we expected. It seems okay for the optimizer to delay propagating attributes from de-refineable functions until you have the export list for the link (and non-expected symbols can be internalized).

----

Sorry... this seems to be pretty off topic, in a way, although maybe not many people know the de-refinement ins and outs so maybe this is useful.

My original points:

- Moving this to the middle end isn't easy (currently, it doesn't have the info it needs, although John might have a proposal for providing it)
- Dropping frontend peepholes in favour of stable IR output seems novel and might deserve a forum discussion (I'd be in favour, personally)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83906



More information about the cfe-commits mailing list