[PATCH] D88979: [InstCombine] combineLoadToOperationType(): don't fold int<->ptr cast into load

[Nuno Lopes via Phabricator via llvm-commits]

nlopes accepted this revision.
nlopes added a comment.

In D88979#2323940 <https://reviews.llvm.org/D88979#2323940>, @lebedev.ri wrote:

> In D88979#2323935 <https://reviews.llvm.org/D88979#2323935>, @nikic wrote:
>
>> LGTM
>
> @nlopes does this look good to you?
>
>> Looking through other uses of isNoopCast(), I don't think it makes sense to push this change into it, as many other usages do need it to work with ptrtoint/inttoptr (some of them using it specifically for them). The comment above the function indicates that "no-op" is to be understood as "generates no code" here. Possibly it could do with a rename.
>
> I think i don't agree with you there.
> I agree with @nlopes, the end goal will be to basically disallow fusing of `inttoptr`/`ptrtoint` into loads, 
> disallow dropping inttoptr-of-ptrtoint/ptrtoint-of-inttoptr, etc.
> And all that eventually boils down to updating `CastInst::isNoopCast()`/`CastInst::isEliminableCastPair()`.

I'm ok with this change, and I agree it's a good step forwards. Thanks!
My only concern was that it seems this code will need further changes going forward (maybe even revert this change?) and I wouldn't want us to forget to revisit this code if/when needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88979