lenary added a comment.
One query, but this patch is starting to look good.
I'm not a libunwind expert - it would be good to have one of the libunwind
contributors look over this patch yet. Can you add one as a reviewer?
================
Comment at: libunwind/src/Registers.hpp:3677
+ case UNW_RISCV_X31:
+ return "t6";
+ case UNW_RISCV_F0:
----------------
Good catch, thanks!
================
Comment at: libunwind/src/Registers.hpp:3756
+inline double Registers_riscv::getFloatRegister(int regNum) const {
+#ifdef __riscv_float_abi_double
+ assert(validFloatRegister(regNum));
----------------
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`.
================
Comment at: libunwind/src/UnwindCursor.hpp:999
+#if defined (_LIBUNWIND_TARGET_RISCV)
+ int stepWithCompactEncoding(Registers_riscv &) {
+ return UNW_EINVAL;
----------------
mhorne wrote:
> lenary wrote:
> > Nit: This should be formatted like the version for `Registers_sparc` above.
> `Registers_sparc` is the outlier in this case, I've formatted it as all the
> other architectures use. If you'd like I could fix the SPARC formatting in
> this patch.
Ah, no worries then. Don't update the sparc formatting in this patch.
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
