luismarques added inline comments.
================
Comment at: libunwind/src/Registers.hpp:3545
+ void setSP(uint64_t value) { _registers[2] = value; }
+ uint64_t getIP() const { return _registers[1]; }
+ void setIP(uint64_t value) { _registers[1] = value; }
----------------
`x1` is the return address. Is this the the `IP` to which we unwind to? How do
we get the original `ra` value at that unwind point?
I vaguely recall seeing this IP/x1 pattern in another unwind library, so it's
probably correct, but it would be good to confirm and document such details.
================
Comment at: libunwind/src/Registers.hpp:3756
+inline double Registers_riscv::getFloatRegister(int regNum) const {
+#ifdef __riscv_float_abi_double
+ assert(validFloatRegister(regNum));
----------------
luismarques wrote:
> lenary wrote:
> > mhorne wrote:
> > > 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`.
> > I had a discussion with @asb about this. The ISA vs ABI issue in RISC-V is
> > complex. The TL;DR is we both think you need to be using `__riscv_flen ==
> > 64` here.
> >
> > The reason for this is that if you compile with `-march=rv64imfd` but
> > `-mabi=lp64`, the architecture still has floating point registers, it just
> > does not use the floating-point calling convention. This means there are
> > still `D`-extension instructions in the stream of instructions, just that
> > "No floating-point registers, if present, are preserved across calls." (see
> > [[
> > https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md#integer-calling-convention
> > | psABI Integer Calling Convention ]]) This effectively means that with
> > this combination, `f0-f31` are treated exactly the same as `t0-t6`, and so
> > should be able to be restored when unwinding. It is not necessarily the
> > case that with a soft float ABI, `f0-f31` are not used at all. This is
> > similar to ARM's `soft` vs `softfp` calling conventions.
> >
> > The expectation is that if you are compiling your programs with a specific
> > `-march`, then you should be compiling your runtime libraries with the same
> > `-march`. Eventually there should be enough details in the ELF file to
> > allow you to ensure both `-march` and `-mabi` match when linking programs,
> > but support for this is not widespread.
> A soft-float *ABI* doesn't mean that FPRs aren't used at all, it means that
> floating-point arguments aren't passed in the floating-point registers. From
> a quick Google search I got the impression that `__mips_hard_float` was used
> for a mips softfloat target (i.e. without hardware floating-point support,
> not for a soft-float ABI), so that's probably not a comparable example.
I just saw @lenary's reply. I agree with his more detailed analysis.
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
