[PATCH] D112016: [IR] Introduce load assume operand bundle

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added a comment.

In D112016#3073861 <https://reviews.llvm.org/D112016#3073861>, @jdoerfert wrote:

> In D112016#3073705 <https://reviews.llvm.org/D112016#3073705>, @nikic wrote:
>
>> In D112016#3073235 <https://reviews.llvm.org/D112016#3073235>, @jdoerfert wrote:
>>
>>> In D112016#3072158 <https://reviews.llvm.org/D112016#3072158>, @nikic wrote:
>>>
>>>> Thus the "as long as the load is speculatable" caveat. It works if you add a `dereferenceable(1)` attribute. Though now that I think about this, I have no idea why speculatability is even a requirement for ephemeral values -- shouldn't side-effect freedom be sufficient? In that case your example would work without the `dereferenceable(1)`.
>>>
>>> I think side-effect free is sufficient, though, haven't thought long about it. (When you say deref(1) you mean deref(sizeof(access)), right?)
>>
>> Right, this was referring to @aeubanks' particular example, which happened to use access size 1.
>>
>> I gave this a try (https://gist.github.com/nikic/531267d972ce71edf3896e25bc50456a) and it seems to work fine (i.e. no test failures). The precise condition would be `wouldInstructionBeTriviallyDead()`, which we can approximate by `!mayHaveSideEffect() && !isTerminator()`.
>
> It seems we want to do that for sure, which means we can (probably should) hold of with this patch for now, right?

there are still many places that don't use ephemerality to count instructions, e.g. the ThinLTO instruction import limit, so I think this still has value
unless you think it'd make more sense to update all the other places, like the ThinLTO instruction import limit, to also use this sort of cost modelling


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112016