nikic 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: > >> 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 agree with the general goal -- my point here is that changing isNoopCast() may not be the way to achieve that, because at least some of the current usages do need to include ptr/int casts, and can include them safely (not all usages result in type punning). In fact, we already have a way to write "isNoopCast() without ptrtoint or inttoptr": `isa<BitCast>`. This didn't quite click before.... You might want to replace `CI->isNoopCast(DL) && LI.getType()->isPtrOrPtrVectorTy() == CI->getDestTy()->isPtrOrPtrVectorTy())` here with just `isa<BitCast>(CI)`. 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
