[PATCH] D134308: AtomicExpand: Use llvm.ptrmask instead of ptrtoint

[James Y Knight via Phabricator via llvm-commits]

jyknight accepted this revision.
jyknight added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for the discussion.

I'd appreciate if you modify the commit to have a clearer description before pushing.



================
Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:707
+  Type *WordPtrType = PMV.WordType->getPointerTo(PtrTy->getAddressSpace());
+  if (!PtrTy->isOpaquePointerTy())
+    PMV.AlignedAddr =
----------------
arsenm wrote:
> jyknight wrote:
> > This "if" seems extraneous -- Builder.CreateBitCast is already a no-op internally if it's asked to cast between opaque pointers, right?
> Yes, but when opaque pointers are removed, how will we track down all the unnecessary CreateBitCast calls?
I'd start by looking at all the calls to `Type::getPointerTo(AS)`, `Type::get*PtrTy(...)`, `PointerType::get(Ty, AS)`, etc. Many such calls can probably be removed entirely, along with their use, being used only as input to some cast creation. Others would still be required, and changed to `PointerType::get(Ctx, AS)` instead. I suspect that'll get rid of most of the redundant casts as a side-effect.

Then, I'd make a temporary local modification where attempting a no-op cast from ptr to ptr will assert-fail, run tests, and examine all the failure locations, to see which other casts ought to be removed.


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

https://reviews.llvm.org/D134308