[PATCH] D18619: [AArch64] Combine callee-save and local stack SP adjustment instructions.

[Tim Northover via llvm-commits]

t.p.northover added inline comments.

================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:324
@@ +323,3 @@
+  unsigned NewOpc;
+  bool NewIsDec = false;
+  bool NewIsUnscaled = false;
----------------
Perhaps use MBBI->isStore()? No obvious way to infer NewIsUnscaled, unfortunately.

================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:395-397
@@ +394,5 @@
+// combined SP bump by adding the local stack size to the stack offsets.
+static MachineBasicBlock::iterator fixupCalleeSaveRestoreStackOffsetAndIncDec(
+    MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI, DebugLoc DL,
+    const TargetInstrInfo *TII, unsigned LocalStackSize) {
+
----------------
I think this function could just be

    static void fixupCalleeSaveRestoreStackOffsetIncAndDec(MachineInstr *MI, unsigned LocalStackSize) {
      MachineOperand &MO = MI->getOperand(MI->getNumExplicitOperands() - 1);
      assert(LocalStackSize % 8 == 0);
      MO.setImm(MO.getImm() + LocalStackSize/8);
    }

But you probably made the right call in `convertCalleeSaveRestoreToSPPrePostIncDec`: the pre/post versions have extra operands that would get messy.

================
Comment at: lib/Target/AArch64/AArch64RegisterInfo.cpp:391-394
@@ -390,3 +390,6 @@
   // Modify MI as necessary to handle as much of 'Offset' as possible
-  Offset = TFI->resolveFrameIndexReference(MF, FrameIndex, FrameReg);
+  bool ForceSP = MI.getFlag(MachineInstr::FrameSetup) ||
+                 MI.getFlag(MachineInstr::FrameDestroy);
+  Offset = TFI->resolveFrameIndexReference(MF, FrameIndex, FrameReg,
+                                           /*PreferFP=*/false, ForceSP);
   if (rewriteAArch64FrameIndex(MI, FIOperandNum, FrameReg, Offset, TII))
----------------
Is this left-over from when you tried to convert spill/restoreCalleeSavedRegisters to use "addFrameIndex"? I can't see the need for it in the present scheme (no FrameSetup/FrameDestroy instructions seem to be created with a FrameIndex operand).


http://reviews.llvm.org/D18619