lebedev.ri added a comment. @nlopes @nikic thank you for the review!
In D88979#2323948 <https://reviews.llvm.org/D88979#2323948>, @nlopes wrote: > 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. Once those methods are fixed, sure, the uses will need dead code cleanup. 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