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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
