[PATCH] D53490: [MIR] Provide a default alignment for stack objects

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added inline comments.


================
Comment at: lib/CodeGen/MIRParser/MIRParser.cpp:574-578
+  // Try to get the default stack alignment from the IR function attrs.
+  if (F.hasFnAttribute(Attribute::StackAlignment))
+    return F.getFnStackAlignment();
+  else
+    return MF.getSubtarget().getFrameLowering()->getStackAlignment();
----------------
This isn't correct since both of these are the alignment of the stack frame itself, not a default alignment of the objects in the frame. You're right that alloca can choose an alignment when it isn't known (it's worth mentioning that this isn't true of other missing alignment information). However, the chosen alignment is necessarily a target decision. Aside from varying between targets, it can be also be context sensitive and varies between types and other things (e.g. fixed location stack frame objects generally don't get to chose, they're specified by the ABI).

Most importantly though, serializing+deserializing via MIR shouldn't change the alignment. With this code present, we would get different behaviour if we pass MIR from one pass to the next directly compared to if we write it to an intermediate file and read it back. I think we should fix AArch64 to account for missing alignment information instead.


https://reviews.llvm.org/D53490