[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