[PATCH] D101264: [IR] Optimize mayBeDerefined for known linkages. NFC

Mon Apr 26 11:49:14 PDT 2021

dblaikie added a comment.

In D101264#2715563 <https://reviews.llvm.org/D101264#2715563>, @MaskRay wrote:

> In D101264#2715553 <https://reviews.llvm.org/D101264#2715553>, @dblaikie wrote:
>
>> Is this functionally changing behavior? (looks like it is, because the 'false' return could currently be overriden by the SemanticInterposable module metadata, resulting in a 'true' return even for these linkages?) If it is a functional change it should have test coverage at least.
>
> No. InternalLinkage and PrivateLinkage imply dso_local. dso_preemptable AppendingLinkage doesn't make sense.

Ah, right - I see in isInterposable. Makes sense.

Is this patch valuable, though - it duplicates some logic from isInterposable which could then become divergent on subsequent code changes? Duplicating logic is generally undesirable for that reason.

>> (though I think it's unfortunate that this would make the noipa work more involved & add more slices of behavior that could be incorrectly used)
>
> The noipa work needs a helper, not reusing the existing `isDefinitionExact`.

I fairly strongly disagree with this - because I think going that way is likely to cause noipa to not be correctly supported in the future when someone adds another optimization, etc, and maybe fixes the interposable case but misses optnone or other cases.

I'm happy renaming those functions - and if there are users that really do need "isInterposable" or "isDefinitionExact" as distinct from this renamed function that includes optnone, I'd like to see those/discuss them.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101264