[PATCH] D104641: [LICM] Strip context sensitive attributes after call hoisting

Mon Jun 21 20:12:50 PDT 2021

anna added a comment.

In D104641#2831539 <https://reviews.llvm.org/D104641#2831539>, @asbirlea wrote:

> It may be too much to overload the existing APIs, since this applies only to calls (e.g. in GVNHoist, metadata is dropped for geps, this would be unnecessary). So either doing the two calls independently as in this patch or add another API that does both. 
> I don't have strong opinions either way.
>
> Is it possible this patch applies at the call in `SimplifyCFG.cpp:1092` and `:2487` and at `hoistAllInstructionsInto` in `Local.cpp` (also used by SimplifyCFG)? If the number of use cases is large enough, an API with both might make sense.

It looks like both cases in SimplifyCFG and `hoistAllInstructionsInto` are applicable (will try constructing test cases). Also, another spot is `makeLoopInvariant` in Analysis/LoopInfo.cpp. There we can make a `readnone` speculatable call loop-invariant (and hoist it to specified point outside loop).

> Side note, I believe there are cases where metadata must be dropped for correctness as well.

Ah, I meant metadata can be dropped unconditionally and that will not impact correctness. Performance can be affected if we dropped metadata that could be retained. 
Whereas for attributes, if we drop them unconditionally, it can affect correctness.

I agree to your point - actually, in most cases, we drop metadata to preserve correctness.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104641