[PATCH] D114246: [RISCV] Fix a bug in RISCVFrameLowering.

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck accepted this revision.
frasercrmck added a comment.
This revision is now accepted and ready to land.

LGTM other than nits. Thanks for the fix!



================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:240
 
-  return MFI.hasVarSizedObjects() && TRI->hasStackRealignment(MF);
+  // If we do not reserve stack space for out-going arguments in prologue,
+  // we will adjust the stack pointer before call instruction. After the
----------------
nit: `outgoing`


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:242
+  // we will adjust the stack pointer before call instruction. After the
+  // adjustment, we could not use $SP to access the stack objects for the
+  // arguments. Instead, use $BP to access these stack objects.
----------------
nit: `can not use`
nit: our other comments in this file don't use dollars when referencing `SP`/`BP`.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:245
+  return (MFI.hasVarSizedObjects() ||
+          (!hasReservedCallFrame(MF) && (MFI.getMaxCallFrameSize() != 0))) &&
+         TRI->hasStackRealignment(MF);
----------------
You don't need parens around `MFI.getMaxCallFrameSize() != 0`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114246