[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to S11

Fangrui Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 22 14:49:13 PDT 2023


MaskRay added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:58
 
   const auto *RVFI = MF.getInfo<RISCVMachineFunctionInfo>();
   if (RVFI->useSaveRestoreLibCalls(MF)) {
----------------
paulkirth wrote:
> craig.topper wrote:
> > paulkirth wrote:
> > > craig.topper wrote:
> > > > Can you add a FIXME here? Using x27 should hopefully remove this restriction
> > > I've added the FIXME, but do we just want to remove this now? or is there some issues or chang under review I can ref in the FIXME?
> > We can remove it, but we should add a test for it.
> That makes sense. Thanks.
Nit: Drop the period while updating the message. See https://llvm.org/docs/CodingStandards.html#error-and-warning-messages


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:116
     Ctx.diagnose(DiagnosticInfoUnsupported{
-        MF.getFunction(), "x18 not reserved by user for Shadow Call Stack."});
-    return;
-  }
-
-  const auto *RVFI = MF.getInfo<RISCVMachineFunctionInfo>();
-  if (RVFI->useSaveRestoreLibCalls(MF)) {
-    Ctx.diagnose(DiagnosticInfoUnsupported{
-        MF.getFunction(),
-        "Shadow Call Stack cannot be combined with Save/Restore LibCalls."});
+        MF.getFunction(), "x27 not reserved by user for Shadow Call Stack."});
     return;
----------------
Nit: Drop the period while updating the message. See https://llvm.org/docs/CodingStandards.html#error-and-warning-messages


================
Comment at: llvm/test/CodeGen/RISCV/saverestore-scs.ll:1
+; Check that shadow call stack doesn't interfere with save/restore
+
----------------
`;; ` appears a useful prefix to highlight non-RUN-non-CHECK comments. It also makes it clear `Check` is not a typo for `CHECK:` when we have some automation tools to detect stale CHECK lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146463



More information about the cfe-commits mailing list