[PATCH] D51030: [AArch64] Optimise load(adr address) to ldr address

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Target/AArch64/AArch64InstrInfo.td:1896
+def alignedload : PatFrag<(ops node:$ptr), (load node:$ptr), [{
+  return cast<LoadSDNode>(N)->getAlignment() >= 4;
+}]>;
----------------
dmgreen wrote:
> efriedma wrote:
> > Checking the alignment of the load isn't going to do the right thing in general; you need to check that the global is aligned.  (Normally, a misaligned load is UB, but this would turn it into a link error.)
> OK thanks. Can you give more details on the difference? Is the load's alignment wrong or inefficient? Or do you mean that in the event that load's alignment is wrong in a way that is UB, it needs to not be link error?
> 
> I would expect if we had an aligned global, <something> (instsimplify maybe?) would set the alignment on the load to that of the global. And we are using the alignment for a ldr here, so it coming from a load at least makes sense in that way.
> 
> There are other examples of aligned load patterns like this, such as alignedload16 from arm or alignedload from x86. They may not be expecting to be acting on the same global data though.
> Or do you mean that in the event that load's alignment is wrong in a way that is UB, it needs to not be link error?

I mean this: we can't reliably prevent optimizations that clone code, like inlining, from introducing UB, so UB can't be a link error.

The requirement here is different from alignedload because normally aligned loads only fault (or load incorrect data) if you violate the alignment requirement; the code still links.  For example, on x86, "movapd 1, %xmm0" is a valid instruction, even though it will always fault.


https://reviews.llvm.org/D51030