[PATCH] D134323: AtomicExpand: Avoid some operations if the atomic is overaligned

[James Y Knight via Phabricator via llvm-commits]

jyknight added inline comments.


================
Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:701
 
-  PMV.AlignedAddrAlignment = Align(MinWordSize);
+  if (AddrAlign < MinWordSize) {
+    IntegerType *IntTy = DL.getIntPtrType(Ctx, PtrTy->getAddressSpace());
----------------
arsenm wrote:
> jyknight wrote:
> > Shouldn't this folding get handled by the IR folder already? Why do we need to special case it here?
> I don't follow. This check switches the path to avoid creating the ptrtoint
What I meant is that when the alignment of Addr is high enough, this could theoretically be optimized away automatically, based on the known-bits computation of Addr causing PtrLSB to be folded to 0, and then AddrInt ending up with no uses.

However, that was a silly comment on my part: InstSimplifyFolder couldn't remove an instruction with no uses, even if it could optimize away everything else. So the ptrtoint would remain regardless. And, this pass comes late enough in the pass pipeline that we aren't going to clean that up after the fact, so we really do want a special-case like this. OK.


================
Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:723
+    PMV.AlignedAddr = Addr;
+    PMV.ShiftAmt = ConstantInt::getNullValue(PMV.WordType);
   }
----------------
You lost the isLittleEndian case here. I think the best fix would be to minimize the amount of code in the conditional section.

Instead of setting PMV.ShiftAmt here, just set `PtrLSB = ConstantInt::getNullValue(IntTy)` in the else clause, and move the entire PMV.ShiftAmt computation into unconditional code -- letting the constant folder take care of things.


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

https://reviews.llvm.org/D134323