[PATCH] D70401: [WIP][RISCV] Implement ilp32e ABI

[Sam Elliott via Phabricator via llvm-commits]

lenary marked 3 inline comments as done.
lenary added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp:104
+  // so that we don't insert `fp` manipulation code into functions that do not
+  // require it.
+  const MachineFrameInfo &MFI = MF.getFrameInfo();
----------------
jrtc27 wrote:
> Shouldn't this all be done by the generic stack realignment code like any other allocation? Or is the issue because it's _register spills_ not explicit allocas?
Yeah the issue is because it’s register spills. I have a nice long commit message I wrote that I should update the summary with.

Comment updated nonetheless


================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.h:40
 
+  bool requiresVirtualBaseRegisters(const MachineFunction &MF) const override {
+    return true;
----------------
I forgot this was left in after some experimentation I did. Will remove it in the next update. 


================
Comment at: llvm/test/CodeGen/RISCV/callee-saved-fpr32s.ll:26
 define void @callee() nounwind {
+; ILP32-ILP32E-LP64-LABEL: callee:
+; ILP32-ILP32E-LP64:       # %bb.0:
----------------
These check lines are left over from before. will remove


================
Comment at: llvm/test/CodeGen/RISCV/stack-realignment.ll:3
 ; RUN: llc -mtriple=riscv32 -verify-machineinstrs < %s \
-; RUN:   | FileCheck %s -check-prefix=RV32I
+; RUN:   | FileCheck %s -check-prefixes=RV32I,RV32-ILP32
+; RUN: llc -mtriple=riscv32 -target-abi ilp32e -verify-machineinstrs < %s \
----------------
jrtc27 wrote:
> Multiple prefixes is a bad idea with update_llc_test_checks.py, and why is this one done differently from the rest?
It also doesn’t help to avoid duplication here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401