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

[Hongtao Yu via Phabricator via cfe-commits]

hoy added a comment.

In D83906#4187872 <https://reviews.llvm.org/D83906#4187872>, @dexonsmith wrote:

> In D83906#4186887 <https://reviews.llvm.org/D83906#4186887>, @hoy wrote:
>
>> In D83906#4184916 <https://reviews.llvm.org/D83906#4184916>, @dexonsmith wrote:
>>
>>> In D83906#4183453 <https://reviews.llvm.org/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 done and the optimizer may be able to recompute the attributes based on pre-LTO attributes, or at least we can allow IPO to one module only, which should still do a better job than FE does?
>>>
>>> I don't think there's much point in passing anything to LTO. There are very few `linkonce_odr` symbols in LTO, since LTO has the advantage of an export list from the link. Symbols not on the export list are internalized (they're given local linkage).
>>
>> That sounds to me an opportunity to get a broader IPO done precisely in the prelink optimizer, as long as we find a way to tell it the incoming IR has source fidelity. What do you think about idea of introducing a module flag? Maybe it's worth discussing in the forum as a followup of introducing a cc1 flag for a stable IR gen.
>
> I'm not sure I'm following.
>
> The prelink optimizer will already be internalizing (i.e., NOT exporting) these symbols. That should be enough. AFAICT, it's non-LTO pipelines that might have headroom after this is reverted.

By prelink I meant the optimizer that run by Clang. The one run by the linker is usually called the postlink optimizer. As you pointed out, the postlink optimizer will unlikely see lots of `linkonce_odr` because of the internalization done right before it. But the prelink optimizer, which is basically a non-LTO optimizer, will still see them.

We actually didn't see expected wins with AutoFDO thinLTO after disabling this specific FE peephole, which I'm guessing might be due to the lack of such peephole in prelink.

> I'm also not sure what the module flag would be for. If "this module has source fidelity", it won't work, because the gadgets I'm aware of are implemented in function passes (probably `-instcombine`?). A function pass isn't allowed to touch module state. Were you thinking of a different module flag? (But, I repeat, I think LTO pipelines have nothing to worry about anyway.)

One of the common places to promoting an invoke to a call is in `SimplifyCFG` (https://github.com/llvm/llvm-project/blob/207854b07dd9bd0d79add49bc5af17f1aabc752f/llvm/lib/Transforms/Utils/Local.cpp#L2498), which is based on function attributes that are inferred by `InferFunctionAttrsPass`. This pass is the first optimization pass in the pipeline, so as long as we can get that pass right, the downstream optimizations should be safe. The module flag I was thinking of is to inform the `InferFunctionAttrsPass` module pass that the IR it's seeing is non-refined.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83906