[PATCH] D92210: Don't sink ptrtoint/inttoptr sequences into non-noop addrspacecasts.

[Tim Besard via Phabricator via llvm-commits]

maleadt added a comment.

Thanks for the review, I've updated the comment.
FWIW, we have already applied this patch to the LLVM tree used by Julia, and it (unsurprisingly) hasn't resulted in any issues.



================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:5049
+    if (SunkAddr->getType() != Addr->getType()) {
+      // Even though we only considered no-op addrspacecasts,
+      // semantically-meaningful conversions may still be present due to
----------------
arsenm wrote:
> efriedma wrote:
> > I'd state this comment differently:
> > 
> > There are two reasons the types might not match: a no-op addrspacecast, or a ptrtoint/inttoptr pair.  Either way, we emit a ptrtoint/inttoptr pair, to ensure we match the original semantics.
> > 
> > --------
> > 
> > Ideally, we don't want to convert an addrspacecast to a ptrtoint/inttoptr pair; it's semantically valid, but we lose information.  At this point in the pipeline, it doesn't affect that much, but it does hurt alias analysis a bit.  I'm okay with starting with a conservative fix, and revisiting later.
> Ideally we would allow bitcast between different address space pointers with the same size, which would avoid the need for this special case
You mean "two reasons the *address spaces of* the types might not match"?

I'll update a comment and add a TODO about bitcasts between different address space pointers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92210