[libcxx-commits] [PATCH] D80690: [RISCV] Support libunwind for riscv32
kamlesh kumar via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Jun 12 10:52:42 PDT 2020
kamleshbhalui marked an inline comment as done.
kamleshbhalui added inline comments.
Comment at: libunwind/include/__libunwind_config.h:132
# define _LIBUNWIND_TARGET_RISCV 1
-# define _LIBUNWIND_CONTEXT_SIZE 64
+# define _LIBUNWIND_CONTEXT_SIZE (32 * (__riscv_xlen + __riscv_flen) / 64)
# define _LIBUNWIND_CURSOR_SIZE 76
> kito-cheng wrote:
> > kamleshbhalui wrote:
> > > luismarques wrote:
> > > > This doesn't seem correct.
> > > OK so it appears that in principle we could have 4 combinations
> > >
> > > ILP32 + "f" : xlen = 32, flen = 32
> > > ILP32 + "fd" : xlen = 32, flen = 64
> > > LP64 + "f" : xlen = 64, flen = 32
> > > LP64 + "fd" : xlen = 64, flen = 64
> > >
> > > So instead of having fixed size area for each kind of register thought of having packed size.
> > > please point me where it can cause problem?
> > >
> > >
> > I am not family with libunwind and don't know what's _LIBUNWIND_CURSOR_SIZE , but I am curious about does here different value for xlen=32/flen=and xlen=64/flen=0?
> > And I think add an #else + #error to catch any unsupported value would be better, since the flen could be 128 (Q), although LLVM didn't implement yet, but in case we implemented in future we'll know we need fix here.
> You are right that `_LIBUNWIND_CONTEXT_SIZE` is correctly defined.
> Can you please add comments explaining the values for `_LIBUNWIND_CURSOR_SIZE`?
> Personally, sometimes I also like to define constants as e.g. (32 + 32) instead of just 64, to make the structure behind the final value clear, so that may also be an option.
I have generated values for _LIBUNWIND_CURSOR_SIZE from UnwindCursor class size for each configration so that checkfit function becomes true.
static_assert((check_fit<UnwindCursor<A, R>, unw_cursor_t>::does_fit),
"UnwindCursor<> does not fit in unw_cursor_t");
I tried to make sense of these values for commenting , but i could not.
CHANGES SINCE LAST ACTION
More information about the libcxx-commits