Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AArch64] Disable red-zone when lowering Q-reg copy through memory. #94962

Merged
merged 2 commits into from
Jun 11, 2024

Conversation

sdesmalen-arm
Copy link
Collaborator

This was pointed out in PR #93940.

@llvmbot
Copy link
Collaborator

llvmbot commented Jun 10, 2024

@llvm/pr-subscribers-backend-aarch64

Author: Sander de Smalen (sdesmalen-arm)

Changes

This was pointed out in PR #93940.


Full diff: https://github.com/llvm/llvm-project/pull/94962.diff

1 Files Affected:

  • (modified) llvm/lib/Target/AArch64/AArch64FrameLowering.cpp (+8-1)
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index cd532671f5018..65e3bbf4e9c35 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -431,8 +431,15 @@ bool AArch64FrameLowering::canUseRedZone(const MachineFunction &MF) const {
   const AArch64FunctionInfo *AFI = MF.getInfo<AArch64FunctionInfo>();
   uint64_t NumBytes = AFI->getLocalStackSize();
 
+  // If neither NEON or SVE are available, a COPY from one Q-reg to
+  // another requires a spill -> reload sequence. We can do that
+  // using a pre-decrementing store/post-decrementing load, but
+  // if we do so, we can't use the Red Zone.
+  bool LowerQRegCopyThroughMem =
+      !Subtarget.isNeonAvailable() && !Subtarget.hasSVE();
+
   return !(MFI.hasCalls() || hasFP(MF) || NumBytes > RedZoneSize ||
-           getSVEStackSize(MF));
+           getSVEStackSize(MF) || LowerQRegCopyThroughMem);
 }
 
 /// hasFP - Return true if the specified function should have a dedicated frame

// using a pre-decrementing store/post-decrementing load, but
// if we do so, we can't use the Red Zone.
bool LowerQRegCopyThroughMem =
!Subtarget.isNeonAvailable() && !Subtarget.hasSVE();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe check for hasFPARMv8(), so this doesn't impact soft-fp code?

Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sdesmalen-arm sdesmalen-arm merged commit c63a622 into llvm:main Jun 11, 2024
7 checks passed
Lukacma pushed a commit to Lukacma/llvm-project that referenced this pull request Jun 12, 2024
@HerrCai0907 HerrCai0907 mentioned this pull request Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants