[PATCH] D80690: [RISCV] Support libunwind for riscv32

[kamlesh kumar via Phabricator via llvm-commits]

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
----------------
luismarques wrote:
> 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.
i.e.
  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
  https://reviews.llvm.org/D80690/new/

https://reviews.llvm.org/D80690