[PATCH] D64898: Standardize how we check for legality of frame realignment

[Philip Reames via Phabricator via llvm-commits]

reames created this revision.
reames added reviewers: craig.topper, asb, echristo.
Herald added subscribers: bollu, mcrosier.
Herald added a project: LLVM.

We currently have three ways to check whether a stack can be realigned.  TargetFrameInfo, MachineFrameInfo, and TargetRegisterInfo.  This patch attempts to standardize on one of them (TRI) as the source of truth.

This patch does change behaviour in corner-cases.  Previously, an overaligned static alloc in a no-realign-stack function had it's alignment silently ignored.  Now, the frame isn't stack object isn't dynamically aligned, but the pointer w/in it is.  (Test case to follow once I get it cleaned up and I'll rebased.)


Repository:
  rL LLVM

https://reviews.llvm.org/D64898

Files:
  include/llvm/CodeGen/TargetFrameLowering.h
  lib/CodeGen/MachineFunction.cpp
  lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
  lib/CodeGen/TargetRegisterInfo.cpp


Index: lib/CodeGen/TargetRegisterInfo.cpp
===================================================================
--- lib/CodeGen/TargetRegisterInfo.cpp
+++ lib/CodeGen/TargetRegisterInfo.cpp
@@ -434,7 +434,8 @@
 }
 
 bool TargetRegisterInfo::canRealignStack(const MachineFunction &MF) const {
-  return !MF.getFunction().hasFnAttribute("no-realign-stack");
+  return MF.getSubtarget().getFrameLowering()->isStackRealignable() &&
+    !MF.getFunction().hasFnAttribute("no-realign-stack");
 }
 
 bool TargetRegisterInfo::needsStackRealignment(
Index: lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
+++ lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
@@ -84,6 +84,7 @@
   TLI = MF->getSubtarget().getTargetLowering();
   RegInfo = &MF->getRegInfo();
   const TargetFrameLowering *TFI = MF->getSubtarget().getFrameLowering();
+  const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
   unsigned StackAlign = TFI->getStackAlignment();
   DA = DAG->getDivergenceAnalysis();
 
@@ -141,7 +142,7 @@
         // adjustment. For targets that don't realign the stack, don't
         // do this if there is an extra alignment requirement.
         if (AI->isStaticAlloca() &&
-            (TFI->isStackRealignable() || (Align <= StackAlign))) {
+            (TRI->canRealignStack(mf) || (Align <= StackAlign))) {
           const ConstantInt *CUI = cast<ConstantInt>(AI->getArraySize());
           uint64_t TySize = MF->getDataLayout().getTypeAllocSize(Ty);
 
@@ -180,7 +181,6 @@
         ImmutableCallSite CS(&I);
         if (isa<InlineAsm>(CS.getCalledValue())) {
           unsigned SP = TLI->getStackPointerRegisterToSaveRestore();
-          const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
           std::vector<TargetLowering::AsmOperandInfo> Ops =
               TLI->ParseConstraints(Fn->getParent()->getDataLayout(), TRI, CS);
           for (TargetLowering::AsmOperandInfo &Op : Ops) {
Index: lib/CodeGen/MachineFunction.cpp
===================================================================
--- lib/CodeGen/MachineFunction.cpp
+++ lib/CodeGen/MachineFunction.cpp
@@ -159,10 +159,8 @@
     RegInfo = nullptr;
 
   MFInfo = nullptr;
-  // We can realign the stack if the target supports it and the user hasn't
-  // explicitly asked us not to.
-  bool CanRealignSP = STI->getFrameLowering()->isStackRealignable() &&
-                      !F.hasFnAttribute("no-realign-stack");
+
+  bool CanRealignSP = STI->getRegisterInfo()->canRealignStack(*this);
   FrameInfo = new (Allocator) MachineFrameInfo(
       getFnStackAlignment(STI, F), /*StackRealignable=*/CanRealignSP,
       /*ForceRealign=*/CanRealignSP &&
Index: include/llvm/CodeGen/TargetFrameLowering.h
===================================================================
--- include/llvm/CodeGen/TargetFrameLowering.h
+++ include/llvm/CodeGen/TargetFrameLowering.h
@@ -99,7 +99,9 @@
   }
 
   /// isStackRealignable - This method returns whether the stack can be
-  /// realigned.
+  /// realigned on this target.  Note that even for targets which allow stack
+  /// realignment, not all functions are required to.  Most potential callers
+  /// probably want TargetRegisterInfo::canRealignStack(MF) instead.
   bool isStackRealignable() const {
     return StackRealignable;
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D64898.210465.patch
Type: text/x-patch
Size: 3401 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190718/ba3dda1f/attachment.bin>