lebedev.ri added subscribers: t.p.northover, dblaikie. lebedev.ri added a comment.
In D99121#2644223 <https://reviews.llvm.org/D99121#2644223>, @nlopes wrote: > The pointee type in LLVM doesn't really matter. It's even supposed to > disappear one day after the migration is completed. > E.g., i8* and i64* are exactly the same thing: they are pointers to data. Yep. That will be indeed a great to see. > So, I don't understand the motivation for this patch. It doesn't solve the > root cause of the problem (which one btw?). It is indeed temporary until Opaque pointers are here. The problem has been stated last time in D99051 <https://reviews.llvm.org/D99051> by @ruiling: https://godbolt.org/z/x7E1EjWvv, i.e. given the same integer, there can be any number of pointers `inttoptr`'d from it, and passes won't be able to tell that they are identical. @dblaikie @t.p.northover can anyone comment on the Opaque Pointers progress? Is there a checklist somewhere? ================ Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:278-280 + if (firstOp == Instruction::IntToPtr && secondOp == Instruction::BitCast) + Res = 0; + ---------------- ruiling wrote: > If we want to disable int2ptr + bitcast unconditionally, then why not > directly make it in CastInst::isEliminableCastPair()? Because technically that is still legal, much like changing integer types of the `inttoptr`/`ptrtoint`, which also aren't prohibited in `isEliminableCastPair()`. ================ Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:1949-1953 + if (CI.getType() != BytePtrTy) { + Value *P = Builder.CreateIntToPtr(CI.getOperand(0), BytePtrTy); + return new BitCastInst(P, CI.getType()); + } + ---------------- ruiling wrote: > The idea here sounds good to me, but seems causing some new problems for the > regression tests. I am not sure the whether the regressions are easy to fix? FWIW all the problems are temporary until LLVM IR is migrated to typeless pointer types. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99121/new/ https://reviews.llvm.org/D99121 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
