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

hoy added a comment. In D83906#4187872 , @dexonsmith wrote: > In D83906#4186887 , @hoy wrote: > >> In D83906#4184916 , @dexonsmith >> wrote: >> >>> In D83906#4183453

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

dexonsmith added a comment. In D83906#4186887 , @hoy wrote: > In D83906#4184916 , @dexonsmith > wrote: > >> In D83906#4183453 , @hoy wrote: >> >>> Wondering if we can come u

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

hoy added a comment. In D83906#4184916 , @dexonsmith wrote: > In D83906#4183453 , @hoy wrote: > >> Wondering if we can come up with a way to tell the optimizer about that, >> e.g., through a new module flag. When i

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

dexonsmith added a comment. In D83906#4183453 , @hoy wrote: > Wondering if we can come up with a way to tell the optimizer about that, > e.g., through a new module flag. When it comes to LTO, the selection of > linkonce_odr symbols should already been do

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

hoy added a comment. In D83906#4182904 , @dexonsmith wrote: > In D83906#4182847 , @hoy wrote: > >> As far as I know, the optimizer IPO pass that infers function attributes >> (i..e `InferFunctionAttrsPass`) is plac

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

sanjoy added a comment. > I just assume there are more devices out there that we don't know about or > understand. I don't totally understand the broader discussion, but `malloc(4) == nullptr` is another gadget. This is optimized to `false` by LLVM even though at runtime it can be false or tr

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

dexonsmith added a subscriber: sanjoy. dexonsmith added a comment. In 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

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

dexonsmith added a comment. In D83906#4182847 , @hoy wrote: > As far as I know, the optimizer IPO pass that infers function attributes > (i..e `InferFunctionAttrsPass`) is placed at the very beginning of the > optimization pipeline. Does this sound to y

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

rjmccall added a comment. 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? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83906/new

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

dexonsmith added a comment. In D83906#4182777 , @rjmccall wrote: > In D83906#4182287 , @dexonsmith > wrote: > >> - At IRGen time, you know the LLVM attributes have not been adjusted after >> the optimized refined

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

hoy added a comment. In D83906#4182476 , @dexonsmith wrote: > In D83906#4182428 , @hoy wrote: > >> In D83906#4182287 , @dexonsmith >> wrote: >> >>> In C++, you get linkonce_

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

rjmccall added a comment. In 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

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

dexonsmith added a comment. In D83906#4182428 , @hoy wrote: > In D83906#4182287 , @dexonsmith > wrote: > >> In C++, you get linkonce_odr all over the place. It's basically all >> functions that are defined in C++

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

hoy added a comment. In D83906#4182287 , @dexonsmith wrote: > 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. > On the other hand, the frontend knows the t

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

dexonsmith added a comment. In 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 co

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

hoy added a comment. In D83906#4180435 , @dexonsmith wrote: > Oh, de-refining is pretty nifty / evil. This patch has background: > https://reviews.llvm.org/D18634 > > Since 2016, the optimizer is not allowed to do IPA on functions that can be > de-refin

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

rjmccall added a comment. 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 o

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

ahatanak added a comment. clang marks the called function `foo` in p1.cpp as nounwind here: https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CodeGenFunction.cpp#L1284 clang can also mark a function declaration as nounwind based on the information in the source code, for example,

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

dexonsmith added a comment. Oh, de-refining is pretty nifty / evil. This patch has background: https://reviews.llvm.org/D18634 Since 2016, the optimizer is not allowed to do IPA on functions that can be de-refined (such as `linkonce_odr` functions). Here's a hypothetical problematic scenario fo

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

wenlei added a comment. > I wonder if this is just a single example, where there could be various other > (header-related) peepholes that cause similar problems for stable output. > IIRC, the usual Clang approach is to make as-close-to-optimal IR up front, > but maybe in some situations it's de

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

hoy added a comment. > (Also, can the backend safely optimize an `invoke` to a `linkonce_odr` > function that's `nounwind` to a `call`? I thought it couldn't, in case the > function is de-refined to a version that's not `nounwind`. But the frontend > can do it since it has access to the sourc

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

dexonsmith added a comment. In D83906#4179512 , @wlei wrote: > Hi @ahatanak > > We recently hit an issue of inconsistent codegen related with this > optimization. In one build, Clang frontend generates different llvm IRs for > the same function that is o

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

wlei added subscribers: modimo, hoy, wlei. wlei added a comment. Hi @ahatanak We recently hit an issue of inconsistent codegen related with this optimization. In one build, Clang frontend generates different llvm IRs for the same function that is originally from one header file. It turned out t

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGed6b578040a8: [CodeGen] Emit a call instruction instead of an invoke if the called llvm… (authored by ahatanak). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

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

ahatanak updated this revision to Diff 278316. ahatanak marked an inline comment as done. ahatanak added a comment. Update comment in test case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83906/new/ https://reviews.llvm.org/D83906 Files: clan

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

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Looks good to me. Comment at: clang/test/CodeGenObjCXX/os_log.mm:16 // An `invoke` of a `nounwind` callee is simplified to a direct // call by an optimization in llvm. Just

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

ahatanak added a comment. In case it wasn't clear, the calls to `mayThrow()` in the test cases are needed to prevent `TryMarkNoThrow` from annotating the functions with nounwind, which would cause a lot of churn. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

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

ahatanak created this revision. ahatanak added reviewers: vsk, rjmccall, ABataev. ahatanak added a project: clang. Herald added subscribers: ributzka, dexonsmith, jkorous. This fixes cases where an invoke is emitted, despite the called llvm function being marked nounwind, because `CodeGenModule::