[PATCH] D45358: [AArch64] Use FP to access the emergency spill slot

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg added inline comments.


================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:1005
       UseFP = hasFP(MF);
-    } else if (hasFP(MF) && !RegInfo->hasBasePointer(MF) &&
-               !RegInfo->needsStackRealignment(MF)) {
-      // Use SP or FP, whichever gives us the best chance of the offset
-      // being in range for direct access. If the FPOffset is positive,
-      // that'll always be best, as the SP will be even further away.
+    } else if (hasFP(MF) && !RegInfo->needsStackRealignment(MF)) {
       // If the FPOffset is negative, we have to keep in mind that the
----------------
efriedma wrote:
> Could this needsStackRealignment check cause problems similar to the bug you're fixing?
I just looked into this, and in [[ https://github.com/llvm-mirror/llvm/blob/9d8994460a682b144caa27f51a956c8c992a024a/lib/CodeGen/PrologEpilogInserter.cpp#L776 | PEI ]] we don't put the emergency spill slot at the top of the frame if we need stack realignment. We put it at the end so the base pointer will be able to reach it and we won't hit this bug.


================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:1020
+          UseFP = PreferFP;
+        else if (!CanUseBP) // Can't use BP. Forced to use FP.
+          UseFP = true;
----------------
efriedma wrote:
> How did the !CanUseBP work before this patch?
Before, we would use FP if `MFI.hasVarSizedObjects` && `!CanUseBP`. `hasBasePointer` can return false even if we have var sized objects, and that's only when we can tell that we can reach everything with FP.


https://reviews.llvm.org/D45358