[PATCH] D123178: [RISCV] Store/restore RISCVMachineFunctionInfo into MIR YAML file

Fraser Cormack via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 7 02:24:29 PDT 2022


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

LGTM other than a couple of issues to fix up.



================
Comment at: llvm/lib/Target/RISCV/RISCVMachineFunctionInfo.cpp:9
+//
+// This file contains the RISCV implementation of TargetFrameLowering class.
+//
----------------
This comment's incorrect, isn't it? Something like this, I'd imagine:
```
// This file declares RISCV-specific per-machine-function information.
```


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:242
+      static_cast<const yaml::RISCVMachineFunctionInfo &>(MFI);
+  MachineFunction &MF = PFS.MF;
+  MF.getInfo<RISCVMachineFunctionInfo>()->initializeBaseYamlFields(YamlMFI);
----------------
I don't see much value in creating `MF` here since we only use it once. I see it's a copy from AArch64. Just `PFS.MF.getInfo....` would do?


================
Comment at: llvm/test/CodeGen/MIR/RISCV/machine-function-info.mir:1
+# RUN: llc -mtriple=riscv64 -run-pass none %s -o - \
+# RUN:    | FileCheck %s
----------------
I think we can merge these two RUN lines:  they're less than 80 cols combined.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123178



More information about the llvm-commits mailing list