mhorne marked 2 inline comments as done.
mhorne added inline comments.

================
Comment at: libunwind/src/Registers.hpp:3756
+inline double Registers_riscv::getFloatRegister(int regNum) const {
+#ifdef __riscv_float_abi_double
+  assert(validFloatRegister(regNum));
----------------
lenary wrote:
> Is this an ABI or an architecture issue? I'm not sure what other libunwind 
> "backends" do for similar cases.
> 
> The difference is, if I compile libunwind with `-march=rv64g -mabi=lp64`, 
> `__riscv_float_abi_double` is not defined (because you're using a soft-float 
> ABI), but `__riscv_flen == 64` (because the machine does have hardware 
> floating-point registers).
> 
> I'm not sure what the intended behaviour of libunwind is in this case.
> 
> `__riscv_float_abi_double` implies `__riscv_flen >= 64`.
An ABI issue, in my opinion. The unwind frame will always contain space for the 
float registers, but accessing them should be disallowed for soft-float 
configurations as the intent of soft-float is that the FPRs will not be used at 
all. I'd say there is precedent for this in the MIPS implementation, since it 
checks for `defined(__mips_hard_float) && __mips_fpr == 64`.


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

https://reviews.llvm.org/D68362



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to