[PATCH] D80370: [CodeGen] Ensure callers of CreateStackTemporary use sensible alignments

[David Sherwood via Phabricator via llvm-commits]

david-arm marked an inline comment as done.
david-arm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp:893
+  SDValue StackPtr =
+      DAG.CreateStackTemporary(Op.getValueType().getStoreSize(), Align);
   // Emit a store to the stack slot.
----------------
efriedma wrote:
> You changed the alignment of the stack temporary... but you still didn't change the alignment of the load or the store to that stack slot, so they're still wrong.  (Same applies to other changes.)
OK fair enough, but previously the code also didn't specify the same alignment chosen by CreateStackTemporary either. So I didn't realise I had to explicitly add the alignment to the loads/stores. My new NEON test now passes with an alignment of 16 bytes, whereas previously it was 32. Since it just worked I assumed that the StackPtr itself contained the alignment, but I can certainly take a look. I don't mind adding the alignments explicitly to the loads and stores at all if that's needed.


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

https://reviews.llvm.org/D80370