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

[Alex Bradbury via Phabricator via llvm-commits]

asb added a comment.

In https://reviews.llvm.org/D29935#847458, @dylanmckay wrote:

> Are there any tests for the stack slot loading/storing? If there are some in a later patch, that's fine too.


They come later. There should really be a comment in eliminateFrameIndex to indicate that it is a placeholder implementation to allow codegen for conditional branches to be testable.



================
Comment at: lib/Target/RISCV/RISCVRegisterInfo.cpp:78
+  } else {
+    report_fatal_error(
+        "Frame offsets outside of the signed 12-bit range not supported");
----------------
dylanmckay wrote:
> I think it makes more sense for `report_fatal_error` to live outside of the `else` entirely, and then the `else` can be removed as well.
Just to clarify, are you suggesting the following?:
```
if (!isInt<12>(Offset)) {
  report_fatal_error(...)
}

MI.getOperand(FIOperandNum).ChangeToRegister(FrameReg, false);
MI.getOperand(FIOperandNum + 1).ChangeToImmediate(Offset);
return;
```



https://reviews.llvm.org/D29935