Targets: commonize some stack realignment code

[jfb]

Summary

This patch does the following:

• Fix FIXME on needsStackRealignment: it is now shared between multiple targets, implemented in TargetRegisterInfo, and isn't virtual anymore. This will break out-of-tree targets, silently if they used virtual and with a build error if they used override.
• Factor out canRealignStack as a virtual function on TargetRegisterInfo, by default only looks for the no-realign-stack function attribute.

Multiple targets duplicated the same needsStackRealignment code:

• Aarch64.
• ARM.
• Mips almost: had extra DEBUG diagnostic, which the default implementation now has.
• PowerPC.
• WebAssembly.
• x86 almost: has an extra -force-align-stack option, which the default implementation now has.

The default implementation of needsStackRealignment used to just return false. My current patch changes the behavior by simply using the above shared behavior. This affects:

• AMDGPU
• BPF
• CppBackend
• MSP430
• NVPTX
• Sparc
• SystemZ
• XCore
• Out-of-tree targets

This is a breaking change! make check passes.

The only implementation of the virtual function (besides the slight different in x86) was Hexagon (which did MF.getFrameInfo()->getMaxAlignment() > 8), and potentially some out-of-tree targets. Hexagon now uses the default implementation.

needsStackRealignment was being overwritten in <Target>GenRegisterInfo.inc, to return false as the default also did. That was odd and is now gone.

[jfb]

I'm open to changing this patch to keep the previous default behavior, it just seemed simpler to put this out as-is for now and see what folks think.

[jfb]

Should we decide to go with the breaking change, I think we should discuss on the mailing list too.

[sunfish]

This looks good to me. It'd be good to get more eyes on this though.

lib/CodeGen/TargetRegisterInfo.cpp

313 ↗

(On Diff #29606)

I think MIPS' assert makes sense here. If a function "needs" stack realignment, we're going to require that it "can" realign its stack.

[jfb]

• Add warning to the default needsStackRealignment implementation.

[hfinkel]

x86 almost: has an extra force-align-stack option, and is otherwise the same as the default implementation. Should this option be shared by all targets?

Yes, that sounds very useful.

Should we decide to go with the breaking change, I think we should discuss on the mailing list too.

Is this a change for anyone other than Hexagon. Their check seems odd: Are they specifying the right stack alignment. We should find out from them.

[mcrosier]

Adding Kryzsztof in case he's like to comment on the Hexagon side.

[rnk]

This is only a breaking change in the sense that it affects needsStackRealignment() for out of tree targets, right? If not, can you elaborate on what I'm missing?

This looks like a good direction.

lib/CodeGen/TargetRegisterInfo.cpp

315 ↗

(On Diff #29713)

Did the assertion not make it past the test suite? It looks like you added the dbgs() after this comment.

lib/Target/X86/X86RegisterInfo.cpp

473 ↗

(On Diff #29713)

This (and other) hasVarSizedObjects() checks should probably be looking for MFI->hasVarSizedObjects() || MFI->hasOpaqueSPAdjustment() as is done above in hasBasePointer. It catches the case where there is some other reason why SP is useless for addressing local variables, like SP-adjusting inline assembly or (ugh) SEH.

[jfb]

This update should address all comments:

• Add warning to the default needsStackRealignment implementation.
• Move -force-align-stack out of x86, and make it work for all targets as suggested by hfinkel.
• Remove x86 override of needsStackRealignment, and make the default one check for -force-align-stack (as the x86 version used to).
• Remove the Hexagon override of needsStackRealignment: comments on Phabricator seem to agree that the code was wrong, and the default implementation is probably right. All tests still pass.
• needsStackRealignment is now non-virtual. Out-of-tree targets need to update their code and instead implement canRealignStack. This will be a silent breaking change if code used 'virtual', but will not be silent if they used 'override'.

[jfb]

I updated the patch to address all comment, except the two below from @rnk.

I'll update the description to accurately describe the current patch.

lib/CodeGen/TargetRegisterInfo.cpp

315 ↗

(On Diff #29713)

This is a non-fatal debug diagnostic, that'll only appear in a debug build with -debug-only=target-reg-info? It shouldn't hard-fail anything IIUC. Or am I misunderstanding?

lib/Target/X86/X86RegisterInfo.cpp

473 ↗

(On Diff #29713)

Would you mind if I do it in a separate patch? I'm assuming it'll require a test too :)

[kparzysz]

The Hexagon implementation was functionally equivalent to what the new implementation does (minus the attribute checks, etc.). The default stack alignment is 8 and the old function really should have used getStackAlignment, which the new one does. This looks good to me.

[jfb]

Any further comments / improvements?

[rnk]

lgtm

[jfb]

lib/CodeGen/TargetRegisterInfo.cpp

323 ↗

(On Diff #29741)

I discussed this offline with @rnk, and I'll submit a follow-up to make it a hard-failure (not just debug print). I'm making it a follow-up so it's less likely to get reverted.

[jfb]

lib/Target/X86/X86RegisterInfo.cpp

473 ↗

(On Diff #29713)

Follow-up in D11377.

[jfb]

@rnk / @hfinkel you may be interested in this comment about needsStackRealignment's diagnostic. If you agree I'll submit another patch to remove the diagnostic.

llvm/trunk/lib/CodeGen/TargetRegisterInfo.cpp

327

I'm not sure this diagnostic makes sense: x86, ARM and MIPS implement canRealignStack, and a few places in the codebase check needsStackRealignment as one of a few conditions which are supposed to be commutable (in a chain of logical ors), or as a condition in a chain of logical ands where the call to canRealignStack was hoisted to be done unconditionally. When this happens, the code assumes that calling needsStackRealignment on invalid functions will just return false and the other checks will handle things. Adding report_fatal_error to canRealignStack breaks that assumption.

I wrote a patch (attached) which tries to fix all of these assumptions, but the code still fails on all the same tests. For example, ARM's hasFP check eventually ends up calling canRealignStack to decide whether getFrameRegister should return FP or SP. The basic situation seems to be sensible (if you can't realign the stack then your frame register is the stack pointer), but making this warning a hard-failure breaks the code.

Similar situations occur for other tests.

I think the right thing to do here is to remove this diagnostic: in some cases it makes sense, but only the (sometimes indirect) caller of needsStackRealignment knows whether that's the case or not. We could instead add an isFatal parameter, but that seems icky because the caller can be indirect (e.g. the previous hasFP example).

Turning this into report_fatal_error makes the following tests fail:

LLVM :: CodeGen/ARM/2010-06-29-PartialRedefFastAlloc.ll
LLVM :: CodeGen/ARM/2011-08-12-vmovqqqq-pseudo.ll
LLVM :: CodeGen/ARM/2012-01-23-PostRA-LICM.ll
LLVM :: CodeGen/ARM/struct_byval.ll
LLVM :: CodeGen/ARM/struct_byval_arm_t1_t2.ll
LLVM :: CodeGen/ARM/thumb1_return_sequence.ll
LLVM :: CodeGen/ARM/vldm-sched-a9.ll
LLVM :: CodeGen/ARM/vst3.ll
LLVM :: CodeGen/Mips/msa/spill.ll
LLVM :: CodeGen/Mips/no-odd-spreg-msa.ll
LLVM :: CodeGen/Thumb/2009-08-20-ISelBug.ll
LLVM :: CodeGen/Thumb/2011-05-11-DAGLegalizer.ll
LLVM :: CodeGen/Thumb/large-stack.ll
LLVM :: CodeGen/Thumb/long.ll
LLVM :: CodeGen/Thumb2/crash.ll
LLVM :: CodeGen/Thumb2/large-call.ll
LLVM :: CodeGen/X86/avx-intel-ocl.ll
LLVM :: CodeGen/X86/avx-load-store.ll
LLVM :: CodeGen/X86/avx-vzeroupper.ll
LLVM :: CodeGen/X86/avx512-intel-ocl.ll
LLVM :: CodeGen/X86/dynamic-allocas-VLAs.ll
LLVM :: CodeGen/X86/half.ll
LLVM :: CodeGen/X86/musttail-fastcall.ll
LLVM :: CodeGen/X86/pr18846.ll
LLVM :: CodeGen/X86/preserve_allcc64.ll
LLVM :: CodeGen/X86/stack-folding-fp-avx1.ll
LLVM :: CodeGen/X86/stack-folding-int-avx1.ll
LLVM :: CodeGen/X86/stack-folding-int-avx2.ll
LLVM :: CodeGen/X86/stack-folding-xop.ll
LLVM :: CodeGen/X86/unaligned-spill-folding.ll
LLVM :: CodeGen/X86/vec_fp_to_int.ll

needsStackRealignment-fatal12 KBDownload

llvm/trunk/include/llvm/Target/TargetRegisterInfo.h

 #ifndef LLVM_TARGET_TARGETREGISTERINFO_H
 #define LLVM_TARGET_TARGETREGISTERINFO_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineValueType.h"
 #include "llvm/IR/CallingConv.h"
 #include "llvm/MC/MCRegisterInfo.h"
+#include "llvm/Support/CommandLine.h"
 #include <cassert>
 #include <functional>
 
 namespace llvm {
 
 class BitVector;
 class MachineFunction;
 class RegScavenger;
 template<class T> class SmallVectorImpl;
 class VirtRegMap;
 class raw_ostream;
 class LiveRegMatrix;
 
+extern cl::opt<bool> ForceStackAlign;
+
 class TargetRegisterClass {
 public:
   typedef const MCPhysReg* iterator;
   typedef const MCPhysReg* const_iterator;
   typedef const MVT::SimpleValueType* vt_iterator;
   typedef const TargetRegisterClass* const * sc_iterator;
 
   // Instance variables filled by tablegen, do not use!
public:
   }
 
   /// trackLivenessAfterRegAlloc - returns true if the live-ins should be tracked
   /// after register allocation.
   virtual bool trackLivenessAfterRegAlloc(const MachineFunction &MF) const {
     return false;
   }
 
+  /// canRealignStack - true if the stack can be realigned for the target.
+  virtual bool canRealignStack(const MachineFunction &MF) const;
+
   /// needsStackRealignment - true if storage within the function requires the
   /// stack pointer to be aligned more than the normal calling convention calls
-  /// for.
-  virtual bool needsStackRealignment(const MachineFunction &MF) const {
-    return false;
-  }
+  /// for. This cannot be overriden by the target, but canRealignStack can be
+  /// overriden.
+  bool needsStackRealignment(const MachineFunction &MF) const;
 
   /// getFrameIndexInstrOffset - Get the offset from the referenced frame
   /// index in the instruction, if there is one.
   virtual int64_t getFrameIndexInstrOffset(const MachineInstr *MI,
                                            int Idx) const {
     return 0;
   }
 

llvm/trunk/lib/CodeGen/TargetRegisterInfo.cpp

 //===- TargetRegisterInfo.cpp - Target Register Information Implementation ===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the TargetRegisterInfo interface.
 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/Target/TargetRegisterInfo.h"
 #include "llvm/ADT/BitVector.h"
+#include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/VirtRegMap.h"
+#include "llvm/IR/Function.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Target/TargetFrameLowering.h"
+#include "llvm/Target/TargetRegisterInfo.h"
+
+#define DEBUG_TYPE "target-reg-info"
+
+namespace llvm {
+cl::opt<bool>
+    ForceStackAlign("force-align-stack",
+                    cl::desc("Force align the stack to the minimum alignment"
+                             " needed for the function."),
+                    cl::init(false), cl::Hidden);
+} // end namespace llvm
 
 using namespace llvm;
 
 TargetRegisterInfo::TargetRegisterInfo(const TargetRegisterInfoDesc *ID,
                              regclass_iterator RCB, regclass_iterator RCE,
                              const char *const *SRINames,
                              const unsigned *SRILaneMasks,
                              unsigned SRICoveringLanes)
TargetRegisterInfo::getRegAllocationHints(unsigned VirtReg,
   // target probably has a reason for removing the register.
   if (std::find(Order.begin(), Order.end(), Phys) == Order.end())
     return;
 
   // All clear, tell the register allocator to prefer this register.
   Hints.push_back(Phys);
 }
 
+bool TargetRegisterInfo::canRealignStack(const MachineFunction &MF) const {
+  return !MF.getFunction()->hasFnAttribute("no-realign-stack");
+}
+
+bool TargetRegisterInfo::needsStackRealignment(
+    const MachineFunction &MF) const {
+  const MachineFrameInfo *MFI = MF.getFrameInfo();
+  const TargetFrameLowering *TFI = MF.getSubtarget().getFrameLowering();
+  const Function *F = MF.getFunction();
+  unsigned StackAlign = TFI->getStackAlignment();
+  bool requiresRealignment = ((MFI->getMaxAlignment() > StackAlign) ||
+                              F->hasFnAttribute(Attribute::StackAlignment));
+  if (ForceStackAlign || requiresRealignment) {
+    if (canRealignStack(MF))
+      return true;
+    DEBUG(dbgs() << "Can't realign function's stack: " << F->getName() << "\n");
+  }
+  return false;
+}
+
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 void
 TargetRegisterInfo::dumpReg(unsigned Reg, unsigned SubRegIndex,
                             const TargetRegisterInfo *TRI) {
   dbgs() << PrintReg(Reg, TRI, SubRegIndex) << "\n";
 }
 #endif

llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.h

public:
   bool hasBasePointer(const MachineFunction &MF) const;
   unsigned getBaseRegister() const;
 
   // Debug information queries.
   unsigned getFrameRegister(const MachineFunction &MF) const override;
 
   unsigned getRegPressureLimit(const TargetRegisterClass *RC,
                                MachineFunction &MF) const override;
-  // Base pointer (stack realignment) support.
-  bool canRealignStack(const MachineFunction &MF) const;
-  bool needsStackRealignment(const MachineFunction &MF) const override;
 };
 
 } // end namespace llvm
 
 #endif

llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp

if (MFI->hasVarSizedObjects()) {
     if (MFI->getLocalFrameSize() < 256)
       return false;
     return true;
   }
 
   return false;
 }
 
-bool AArch64RegisterInfo::canRealignStack(const MachineFunction &MF) const {
-
-  if (MF.getFunction()->hasFnAttribute("no-realign-stack"))
-    return false;
-
-  return true;
-}
-
-// FIXME: share this with other backends with identical implementation?
-bool
-AArch64RegisterInfo::needsStackRealignment(const MachineFunction &MF) const {
-  const MachineFrameInfo *MFI = MF.getFrameInfo();
-  const AArch64FrameLowering *TFI = getFrameLowering(MF);
-  const Function *F = MF.getFunction();
-  unsigned StackAlign = TFI->getStackAlignment();
-  bool requiresRealignment =
-      ((MFI->getMaxAlignment() > StackAlign) ||
-       F->getAttributes().hasAttribute(AttributeSet::FunctionIndex,
-                                       Attribute::StackAlignment));
-
-  return requiresRealignment && canRealignStack(MF);
-}
-
 unsigned
 AArch64RegisterInfo::getFrameRegister(const MachineFunction &MF) const {
   const AArch64FrameLowering *TFI = getFrameLowering(MF);
   return TFI->hasFP(MF) ? AArch64::FP : AArch64::SP;
 }
 
 bool AArch64RegisterInfo::requiresRegisterScavenging(
     const MachineFunction &MF) const {

llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.h

void getRegAllocationHints(unsigned VirtReg,
                              const VirtRegMap *VRM,
                              const LiveRegMatrix *Matrix) const override;
 
   void updateRegAllocHint(unsigned Reg, unsigned NewReg,
                           MachineFunction &MF) const override;
 
   bool hasBasePointer(const MachineFunction &MF) const;
 
-  bool canRealignStack(const MachineFunction &MF) const;
-  bool needsStackRealignment(const MachineFunction &MF) const override;
+  bool canRealignStack(const MachineFunction &MF) const override;
   int64_t getFrameIndexInstrOffset(const MachineInstr *MI,
                                    int Idx) const override;
   bool needsFrameBaseReg(MachineInstr *MI, int64_t Offset) const override;
   void materializeFrameBaseRegister(MachineBasicBlock *MBB,
                                     unsigned BaseReg, int FrameIdx,
                                     int64_t Offset) const override;
   void resolveFrameIndex(MachineInstr &MI, unsigned BaseReg,
                          int64_t Offset) const override;

llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp

 bool ARMBaseRegisterInfo::canRealignStack(const MachineFunction &MF) const {
   const MachineRegisterInfo *MRI = &MF.getRegInfo();
   const ARMFunctionInfo *AFI = MF.getInfo<ARMFunctionInfo>();
   const ARMFrameLowering *TFI = getFrameLowering(MF);
   // We can't realign the stack if:
   // 1. Dynamic stack realignment is explicitly disabled,
   // 2. This is a Thumb1 function (it's not useful, so we don't bother), or
   // 3. There are VLAs in the function and the base pointer is disabled.
-  if (MF.getFunction()->hasFnAttribute("no-realign-stack"))
+  if (!TargetRegisterInfo::canRealignStack(MF))
     return false;
   if (AFI->isThumb1OnlyFunction())
     return false;
   // Stack realignment requires a frame pointer.  If we already started
   // register allocation with frame pointer elimination, it is too late now.
   if (!MRI->canReserveReg(getFramePointerReg(MF.getSubtarget<ARMSubtarget>())))
     return false;
   // We may also need a base pointer if there are dynamic allocas or stack
   // pointer adjustments around calls.
   if (TFI->hasReservedCallFrame(MF))
     return true;
   // A base pointer is required and allowed.  Check that it isn't too late to
   // reserve it.
   return MRI->canReserveReg(BasePtr);
 }
 
 bool ARMBaseRegisterInfo::
-needsStackRealignment(const MachineFunction &MF) const {
-  const MachineFrameInfo *MFI = MF.getFrameInfo();
-  const ARMFrameLowering *TFI = getFrameLowering(MF);
-  const Function *F = MF.getFunction();
-  unsigned StackAlign = TFI->getStackAlignment();
-  bool requiresRealignment = ((MFI->getMaxAlignment() > StackAlign) ||
-                              F->hasFnAttribute(Attribute::StackAlignment));
-
-  return requiresRealignment && canRealignStack(MF);
-}
-
-bool ARMBaseRegisterInfo::
 cannotEliminateFrame(const MachineFunction &MF) const {
   const MachineFrameInfo *MFI = MF.getFrameInfo();
   if (MF.getTarget().Options.DisableFramePointerElim(MF) && MFI->adjustsStack())
     return true;
   return MFI->hasVarSizedObjects() || MFI->isFrameAddressTaken()
     || needsStackRealignment(MF);
 }
 

llvm/trunk/lib/Target/Hexagon/HexagonRegisterInfo.h

public:
   }
 
   /// Returns true. Spill code for predicate registers might need an extra
   /// register.
   bool requiresFrameIndexScavenging(const MachineFunction &MF) const override {
     return true;
   }
 
-  bool needsStackRealignment(const MachineFunction &MF) const override;
-
   /// Returns true if the frame pointer is valid.
   bool useFPForScavengingIndex(const MachineFunction &MF) const override;
 
   bool trackLivenessAfterRegAlloc(const MachineFunction &MF) const override {
     return true;
   }
 
   // Debug information queries.

llvm/trunk/lib/Target/Hexagon/HexagonRegisterInfo.cpp

 
 bool
 HexagonRegisterInfo::useFPForScavengingIndex(const MachineFunction &MF) const {
   const HexagonFrameLowering *TFI = getFrameLowering(MF);
   return TFI->hasFP(MF);
 }
 
 
-bool
-HexagonRegisterInfo::needsStackRealignment(const MachineFunction &MF) const {
-  const MachineFrameInfo *MFI = MF.getFrameInfo();
-  return MFI->getMaxAlignment() > 8;
-}
-
-
 unsigned HexagonRegisterInfo::getFirstCallerSavedNonParamReg() const {
   return Hexagon::R6;
 }
 
 
 #define GET_REGINFO_TARGET_DESC
 #include "HexagonGenRegisterInfo.inc"

llvm/trunk/lib/Target/Mips/MipsRegisterInfo.h

public:
   void eliminateFrameIndex(MachineBasicBlock::iterator II,
                            int SPAdj, unsigned FIOperandNum,
                            RegScavenger *RS = nullptr) const override;
 
   void processFunctionBeforeFrameFinalized(MachineFunction &MF,
                                        RegScavenger *RS = nullptr) const;
 
   // Stack realignment queries.
-  bool canRealignStack(const MachineFunction &MF) const;
-
-  bool needsStackRealignment(const MachineFunction &MF) const override;
+  bool canRealignStack(const MachineFunction &MF) const override;
 
   /// Debug information queries.
   unsigned getFrameRegister(const MachineFunction &MF) const override;
 
   /// \brief Return GPR register class.
   virtual const TargetRegisterClass *intRegClass(unsigned Size) const = 0;
 
 private:
   virtual void eliminateFI(MachineBasicBlock::iterator II, unsigned OpNo,
                            int FrameIndex, uint64_t StackSize,
                            int64_t SPOffset) const = 0;
 };
 
 } // end namespace llvm
 
 #endif

llvm/trunk/lib/Target/Mips/MipsRegisterInfo.cpp

getFrameRegister(const MachineFunction &MF) const {
   if (Subtarget.inMips16Mode())
     return TFI->hasFP(MF) ? Mips::S0 : Mips::SP;
   else
     return TFI->hasFP(MF) ? (IsN64 ? Mips::FP_64 : Mips::FP) :
                             (IsN64 ? Mips::SP_64 : Mips::SP);
 }
 
 bool MipsRegisterInfo::canRealignStack(const MachineFunction &MF) const {
+  // Avoid realigning functions that explicitly do not want to be realigned.
+  // Normally, we should report an error when a function should be dynamically
+  // realigned but also has the attribute no-realign-stack. Unfortunately,
+  // with this attribute, MachineFrameInfo clamps each new object's alignment
+  // to that of the stack's alignment as specified by the ABI. As a result,
+  // the information of whether we have objects with larger alignment
+  // requirement than the stack's alignment is already lost at this point.
+  if (!TargetRegisterInfo::canRealignStack(MF))
+    return false;
+
   const MipsSubtarget &Subtarget = MF.getSubtarget<MipsSubtarget>();
   unsigned FP = Subtarget.isGP32bit() ? Mips::FP : Mips::FP_64;
   unsigned BP = Subtarget.isGP32bit() ? Mips::S7 : Mips::S7_64;
 
   // Support dynamic stack realignment only for targets with standard encoding.
   if (!Subtarget.hasStandardEncoding())
     return false;
 
   // We can't perform dynamic stack realignment if we can't reserve the
   // frame pointer register.
   if (!MF.getRegInfo().canReserveReg(FP))
     return false;
 
   // We can realign the stack if we know the maximum call frame size and we
   // don't have variable sized objects.
   if (Subtarget.getFrameLowering()->hasReservedCallFrame(MF))
     return true;
 
   // We have to reserve the base pointer register in the presence of variable
   // sized objects.
   return MF.getRegInfo().canReserveReg(BP);
 }
-
-bool MipsRegisterInfo::needsStackRealignment(const MachineFunction &MF) const {
-  const MipsSubtarget &Subtarget = MF.getSubtarget<MipsSubtarget>();
-  const MachineFrameInfo *MFI = MF.getFrameInfo();
-
-  bool CanRealign = canRealignStack(MF);
-
-  // Avoid realigning functions that explicitly do not want to be realigned.
-  // Normally, we should report an error when a function should be dynamically
-  // realigned but also has the attribute no-realign-stack. Unfortunately,
-  // with this attribute, MachineFrameInfo clamps each new object's alignment
-  // to that of the stack's alignment as specified by the ABI. As a result,
-  // the information of whether we have objects with larger alignment
-  // requirement than the stack's alignment is already lost at this point.
-  if (MF.getFunction()->hasFnAttribute("no-realign-stack"))
-    return false;
-
-  const Function *F = MF.getFunction();
-  if (F->hasFnAttribute(Attribute::StackAlignment)) {
-#ifdef DEBUG
-    if (!CanRealign)
-      DEBUG(dbgs() << "It's not possible to realign the stack of the function: "
-            << F->getName() << "\n");
-#endif
-    return CanRealign;
-  }
-
-  unsigned StackAlignment = Subtarget.getFrameLowering()->getStackAlignment();
-  if (MFI->getMaxAlignment() > StackAlignment) {
-#ifdef DEBUG
-    if (!CanRealign)
-      DEBUG(dbgs() << "It's not possible to realign the stack of the function: "
-            << F->getName() << "\n");
-#endif
-    return CanRealign;
-  }
-
-  return false;
-}

llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.h

bool isFrameOffsetLegal(const MachineInstr *MI, unsigned BaseReg,
                           int64_t Offset) const override;
 
   // Debug information queries.
   unsigned getFrameRegister(const MachineFunction &MF) const override;
 
   // Base pointer (stack realignment) support.
   unsigned getBaseRegister(const MachineFunction &MF) const;
   bool hasBasePointer(const MachineFunction &MF) const;
-  bool canRealignStack(const MachineFunction &MF) const;
-  bool needsStackRealignment(const MachineFunction &MF) const override;
 };
 
 } // end namespace llvm
 
 #endif

llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.cpp

if (AlwaysBasePointer)
     return true;
 
   // If we need to realign the stack, then the stack pointer can no longer
   // serve as an offset into the caller's stack space. As a result, we need a
   // base pointer.
   return needsStackRealignment(MF);
 }
 
-bool PPCRegisterInfo::canRealignStack(const MachineFunction &MF) const {
-  if (MF.getFunction()->hasFnAttribute("no-realign-stack"))
-    return false;
-
-  return true;
-}
-
-bool PPCRegisterInfo::needsStackRealignment(const MachineFunction &MF) const {
-  const PPCFrameLowering *TFI = getFrameLowering(MF);
-  const MachineFrameInfo *MFI = MF.getFrameInfo();
-  const Function *F = MF.getFunction();
-  unsigned StackAlign = TFI->getStackAlignment();
-  bool requiresRealignment = ((MFI->getMaxAlignment() > StackAlign) ||
-                              F->hasFnAttribute(Attribute::StackAlignment));
-
-  return requiresRealignment && canRealignStack(MF);
-}
-
 /// Returns true if the instruction's frame index
 /// reference would be better served by a base register other than FP
 /// or SP. Used by LocalStackFrameAllocation to determine which frame index
 /// references it should create new base registers for.
 bool PPCRegisterInfo::
 needsFrameBaseReg(MachineInstr *MI, int64_t Offset) const {
   assert(Offset < 0 && "Local offset must be negative");
 

llvm/trunk/lib/Target/WebAssembly/WebAssemblyRegisterInfo.h

public:
   const MCPhysReg *getCalleeSavedRegs(const MachineFunction *MF) const override;
   BitVector getReservedRegs(const MachineFunction &MF) const override;
   void eliminateFrameIndex(MachineBasicBlock::iterator MI, int SPAdj,
                            unsigned FIOperandNum,
                            RegScavenger *RS = nullptr) const override;
 
   // Debug information queries.
   unsigned getFrameRegister(const MachineFunction &MF) const override;
-
-  // Base pointer (stack realignment) support.
-  bool canRealignStack(const MachineFunction &MF) const;
-  bool needsStackRealignment(const MachineFunction &MF) const override;
 };
 
 } // end namespace llvm
 
 #endif

llvm/trunk/lib/Target/WebAssembly/WebAssemblyRegisterInfo.cpp

 WebAssemblyRegisterInfo::getFrameRegister(const MachineFunction &MF) const {
   static const unsigned Regs[2][2] = {
       /*            !isArch64Bit       isArch64Bit      */
       /* !hasFP */ {WebAssembly::SP32, WebAssembly::SP64},
       /*  hasFP */ {WebAssembly::FP32, WebAssembly::FP64}};
   const WebAssemblyFrameLowering *TFI = getFrameLowering(MF);
   return Regs[TFI->hasFP(MF)][TT.isArch64Bit()];
 }
-
-bool WebAssemblyRegisterInfo::canRealignStack(const MachineFunction &MF) const {
-  return !MF.getFunction()->hasFnAttribute("no-realign-stack");
-}
-
-// FIXME: share this with other backends with identical implementation?
-bool WebAssemblyRegisterInfo::needsStackRealignment(
-    const MachineFunction &MF) const {
-  const MachineFrameInfo *MFI = MF.getFrameInfo();
-  const WebAssemblyFrameLowering *TFI = getFrameLowering(MF);
-  const Function *F = MF.getFunction();
-  unsigned StackAlign = TFI->getStackAlignment();
-  bool requiresRealignment =
-      ((MFI->getMaxAlignment() > StackAlign) ||
-       F->getAttributes().hasAttribute(AttributeSet::FunctionIndex,
-                                       Attribute::StackAlignment));
-
-  return requiresRealignment && canRealignStack(MF);
-}

llvm/trunk/lib/Target/X86/X86FrameLowering.cpp

 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Function.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCSymbol.h"
-#include "llvm/Support/CommandLine.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Support/Debug.h"
 #include <cstdlib>
 
 using namespace llvm;
 
-// FIXME: completely move here.
-extern cl::opt<bool> ForceStackAlign;
-
 X86FrameLowering::X86FrameLowering(const X86Subtarget &STI,
                                    unsigned StackAlignOverride)
     : TargetFrameLowering(StackGrowsDown, StackAlignOverride,
                           STI.is64Bit() ? -8 : -4),
       STI(STI), TII(*STI.getInstrInfo()), TRI(STI.getRegisterInfo()) {
   // Cache a bunch of frame-related predicates for this subtarget.
   SlotSize = TRI->getSlotSize();
   Is64Bit = STI.is64Bit();

llvm/trunk/lib/Target/X86/X86RegisterInfo.h

public:
   /// should be considered unavailable at all times, e.g. SP, RA. This is used by
   /// register scavenger to determine what registers are free.
   BitVector getReservedRegs(const MachineFunction &MF) const override;
 
   void adjustStackMapLiveOutMask(uint32_t *Mask) const override;
 
   bool hasBasePointer(const MachineFunction &MF) const;
 
-  bool canRealignStack(const MachineFunction &MF) const;
-
-  bool needsStackRealignment(const MachineFunction &MF) const override;
+  bool canRealignStack(const MachineFunction &MF) const override;
 
   bool hasReservedSpillSlot(const MachineFunction &MF, unsigned Reg,
                             int &FrameIdx) const override;
 
   void eliminateFrameIndex(MachineBasicBlock::iterator MI,
                            int SPAdj, unsigned FIOperandNum,
                            RegScavenger *RS = nullptr) const override;
 

llvm/trunk/lib/Target/X86/X86RegisterInfo.cpp

 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetOptions.h"
 
 using namespace llvm;
 
 #define GET_REGINFO_TARGET_DESC
 #include "X86GenRegisterInfo.inc"
 
-cl::opt<bool>
-ForceStackAlign("force-align-stack",
-                 cl::desc("Force align the stack to the minimum alignment"
-                           " needed for the function."),
-                 cl::init(false), cl::Hidden);
-
 static cl::opt<bool>
 EnableBasePointer("x86-use-base-pointer", cl::Hidden, cl::init(true),
           cl::desc("Enable use of a base pointer for complex stack frames"));
 
 X86RegisterInfo::X86RegisterInfo(const Triple &TT)
     : X86GenRegisterInfo((TT.isArch64Bit() ? X86::RIP : X86::EIP),
                          X86_MC::getDwarfRegFlavour(TT, false),
                          X86_MC::getDwarfRegFlavour(TT, true),
bool X86RegisterInfo::hasBasePointer(const MachineFunction &MF) const {
    // use both the SP and the FP, we need a separate base pointer register.
    bool CantUseFP = needsStackRealignment(MF);
    bool CantUseSP =
        MFI->hasVarSizedObjects() || MFI->hasOpaqueSPAdjustment();
    return CantUseFP && CantUseSP;
 }
 
 bool X86RegisterInfo::canRealignStack(const MachineFunction &MF) const {
-  if (MF.getFunction()->hasFnAttribute("no-realign-stack"))
+  if (!TargetRegisterInfo::canRealignStack(MF))
     return false;
 
   const MachineFrameInfo *MFI = MF.getFrameInfo();
   const MachineRegisterInfo *MRI = &MF.getRegInfo();
 
   // Stack realignment requires a frame pointer.  If we already started
   // register allocation with frame pointer elimination, it is too late now.
   if (!MRI->canReserveReg(FramePtr))
     return false;
 
   // If a base pointer is necessary.  Check that it isn't too late to reserve
   // it.
   if (MFI->hasVarSizedObjects())
     return MRI->canReserveReg(BasePtr);
   return true;
 }
 
-bool X86RegisterInfo::needsStackRealignment(const MachineFunction &MF) const {
-  const MachineFrameInfo *MFI = MF.getFrameInfo();
-  const X86FrameLowering *TFI = getFrameLowering(MF);
-  const Function *F = MF.getFunction();
-  unsigned StackAlign = TFI->getStackAlignment();
-  bool requiresRealignment = ((MFI->getMaxAlignment() > StackAlign) ||
-                              F->hasFnAttribute(Attribute::StackAlignment));
-
-  // If we've requested that we force align the stack do so now.
-  if (ForceStackAlign)
-    return canRealignStack(MF);
-
-  return requiresRealignment && canRealignStack(MF);
-}
-
 bool X86RegisterInfo::hasReservedSpillSlot(const MachineFunction &MF,
                                            unsigned Reg, int &FrameIdx) const {
   // Since X86 defines assignCalleeSavedSpillSlots which always return true
   // this function neither used nor tested.
   llvm_unreachable("Unused function on X86. Otherwise need a test case.");
 }
 
 void

llvm/trunk/test/CodeGen/Generic/ForceStackAlign.ll

+; Check that stack alignment can be forced. Individual targets should test their
+; specific implementation details.
+
+; RUN: llc < %s -force-align-stack -stack-alignment=32 | FileCheck %s
+; CHECK-LABEL: @f
+; CHECK-LABEL: @g
+
+define i32 @f(i8* %p) nounwind {
+entry:
+  %0 = load i8, i8* %p
+  %conv = sext i8 %0 to i32
+  ret i32 %conv
+}
+
+define i64 @g(i32 %i) nounwind {
+entry:
+  br label %if.then
+
+if.then:
+  %0 = alloca i8, i32 %i
+  call void @llvm.memset.p0i8.i32(i8* %0, i8 0, i32 %i, i32 1, i1 false)
+  %call = call i32 @f(i8* %0)
+  %conv = sext i32 %call to i64
+  ret i64 %conv
+}
+
+declare void @llvm.memset.p0i8.i32(i8*, i8, i32, i32, i1) nounwind

llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp

RegisterInfoEmitter::runTargetHeader(raw_ostream &OS, CodeGenTarget &Target,
   OS << "#include \"llvm/Target/TargetRegisterInfo.h\"\n\n";
 
   OS << "namespace llvm {\n\n";
 
   OS << "class " << TargetName << "FrameLowering;\n\n";
 
   OS << "struct " << ClassName << " : public TargetRegisterInfo {\n"
      << "  explicit " << ClassName
-     << "(unsigned RA, unsigned D = 0, unsigned E = 0, unsigned PC = 0);\n"
-     << "  bool needsStackRealignment(const MachineFunction &) const override\n"
-     << "     { return false; }\n";
+     << "(unsigned RA, unsigned D = 0, unsigned E = 0, unsigned PC = 0);\n";
   if (!RegBank.getSubRegIndices().empty()) {
     OS << "  unsigned composeSubRegIndicesImpl"
        << "(unsigned, unsigned) const override;\n"
        << "  unsigned composeSubRegIndexLaneMaskImpl"
        << "(unsigned, unsigned) const override;\n"
        << "  const TargetRegisterClass *getSubClassWithSubReg"
        << "(const TargetRegisterClass*, unsigned) const override;\n";
   }