[PATCH] [AArch64] Add support for dynamically aligning stack objects

Tim Northover t.p.northover at gmail.com
Tue Apr 7 12:22:30 PDT 2015


Hi Kristof,

Thanks for working on this, it's a good feature to have. Just a couple of suggestions:


REPOSITORY
  rL LLVM

================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:367
@@ +366,3 @@
+
+  if (NumBytes && NeedsRealignment) {
+    const unsigned NrBitsToZero = countTrailingZeros(Alignment);
----------------
What do we do if NumBytes == 0 but we need realignment? I'm OK with not handling that case (Clang won't produce an "alignstack=BIG" attribute for AArch64), but we should probably assert that it can't happen somewhere.

================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:374
@@ +373,3 @@
+    //   -- free to use. This is already produced by emitFrameOffset above.
+    // BFI X9, Xzr, 0, log2(maxAlign)
+    //   -- which is an alias for BFM X9, X0, 0, log2(maxAlign)-1
----------------
Any particular reason for using BFI here? I'd have thought AND would be the more natural choice: it only has one register dependency (even if the XZR is a false dependency here) and can directly write to SP (avoiding the copy).

Slightly more difficult to encode, of course.

Either way, could we assert that ScratchSPReg is not SP here: neither instruction permits SP as an input, but refactoring could easily mess up that invariant later on.

http://reviews.llvm.org/D8876

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list