[PATCH] D29935: [RISCV 13/n] Codegen for conditional branches

[Dylan McKay via Phabricator via llvm-commits]

dylanmckay added inline comments.


================
Comment at: lib/Target/RISCV/RISCV.h:26
 class AsmPrinter;
 class RISCVTargetMachine;
+class MCContext;
----------------
Can we make these forward declarations alphabetically ordered?


================
Comment at: lib/Target/RISCV/RISCVInstrInfo.h:39
+                           MachineBasicBlock::iterator MBBI, unsigned SrcReg,
+                           bool isKill, int FrameIndex,
+                           const TargetRegisterClass *RC,
----------------
IsKill should start with a capital letter


================
Comment at: lib/Target/RISCV/RISCVRegisterInfo.cpp:59
                                             RegScavenger *RS) const {
-  report_fatal_error("Subroutines not supported yet");
+  assert(SPAdj == 0 && "Unexpected");
+
----------------
Can we make this assertion message better?


================
Comment at: lib/Target/RISCV/RISCVRegisterInfo.cpp:72
+  if (!TFI->hasFP(MF)) {
+    report_fatal_error("eliminateFrameIndex currently requires hasFP");
+  }
----------------
This sounds less like a fatal error and more like an assertion


================
Comment at: lib/Target/RISCV/RISCVRegisterInfo.cpp:80
+    return;
+  } else {
+    report_fatal_error(
----------------
[No else after return](https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return)


https://reviews.llvm.org/D29935