[PATCH] D107886: [PowerPC] Support huge frame size for PPC64

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCFrameLowering.cpp:629
   int64_t NegFrameSize = -FrameSize;
-  if (!isInt<32>(FrameSize) || !isInt<32>(NegFrameSize))
-    llvm_unreachable("Unhandled stack size!");
----------------
You remove this here but this pre-condition still holds for 32-bit mode. Why not change the condition to only trap in 32-bit mode rather than never?


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3238
+  assert(Register::isPhysicalRegister(Reg) &&
+         "Receiver register should be physical");
+  bool isPPC64 = Subtarget.isPPC64();
----------------
"Receiver" sounds strange. "Target" seems like a more common name.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3246
+        .addImm(Imm >> 16);
+    BuildMI(MBB, MBBI, DL, get(isPPC64 ? PPC::ORI8 : PPC::ORI), Reg)
+        .addReg(Reg, RegState::Kill)
----------------
I am not suggesting we need to re-implement optimal constant materialization here, but it seems trivially easy to only emit the `ORI` variants only if the immediate is non-zero.


================
Comment at: llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp:1495
+      assert(is64Bit && "Huge stack is only supported on PPC64");
+      BuildMI(MBB, II, dl, TII.get(PPC::LIS8) , SReg)
+        .addImm(Offset >> 48);
----------------
Why is this repeated here rather than calling `materializeImmPostRA()` as you do in other places?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107886/new/

https://reviews.llvm.org/D107886