On Sat, 2021-11-20 at 17:34 -0500, Antoni Boucher wrote: > Hi. > Here's the updated patch. > See comments below. > Thanks for your reviews! > > Le jeudi 20 mai 2021 à 16:11 -0400, David Malcolm a écrit : > > On Tue, 2021-05-18 at 20:43 -0400, Antoni Boucher via Gcc-patches > > wrote: > > > Hello. > > > This patch adds support for TLS variables. > > > One thing to fix before we merge it is the libgccjit.map file > > > which > > > contains LIBGCCJIT_ABI_16 instead of LIBGCCJIT_ABI_17. > > > LIBGCCJIT_ABI_16 was added in one of my other patches. > > > Thanks for the review. > > > > > diff --git a/gcc/jit/docs/topics/compatibility.rst > > > b/gcc/jit/docs/topics/compatibility.rst > > > index 239b6aa1a92..d10bc1df080 100644 > > > --- a/gcc/jit/docs/topics/compatibility.rst > > > +++ b/gcc/jit/docs/topics/compatibility.rst > > > @@ -243,3 +243,12 @@ embedding assembler instructions: > > > * :func:`gcc_jit_extended_asm_add_input_operand` > > > * :func:`gcc_jit_extended_asm_add_clobber` > > > * :func:`gcc_jit_context_add_top_level_asm` > > > + > > > +.. _LIBGCCJIT_ABI_17: > > > + > > > +``LIBGCCJIT_ABI_17`` > > > +----------------------- > > > +``LIBGCCJIT_ABI_17`` covers the addition of an API entrypoint to > > > set > > > the > > > +thread-local storage model of a variable: > > > + > > > + * :func:`gcc_jit_lvalue_set_tls_model` > > > > Sorry about the delay in reviewing patches. > > > > Is there a summary somewhere of the various outstanding patches and > > their associated ABI versions? Are there dependencies between the > > patches? > > The list of patches is there: > https://github.com/antoyo/libgccjit-patches but I don't keep them up- > to-date. > If that would help you, I could add a README to tell what is the new > ABI version for each patch. > I believe there might be some patches that depend on a previous one.
That's not needed; I think all I need to know is what the next patch you need me to look at is (FWIW I'm about to go on vacation for a week) [...snip...] > > > > > > + > > > +void > > > +create_code (gcc_jit_context *ctxt, void *user_data) > > > +{ > > > + /* Let's try to inject the equivalent of: > > > + > > > + _Thread_local int foo; > > > + */ > > > + gcc_jit_type *int_type = > > > + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT); > > > + > > > + gcc_jit_lvalue *foo = > > > + gcc_jit_context_new_global ( > > > + ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED, int_type, "foo"); > > > + gcc_jit_lvalue_set_tls_model (foo, > > > GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC); > > > > How many of the different enum values can be supported? How > > target- > > dependent is this? > > I'm not sure what you mean here. Are you asking that I test all the > different enum values? That would be ideal, but I don't think it's necessary. > The tls_model enum is defined in gcc/coretypes.h and does not seem to > change depending on the target. Maybe there are checks elsewhere for > that, though. It might be that some targets only support some modes; I don't know. [...snip...] Thanks for the updated patch. It looks good to push to trunk once the earlier ones are in place, though as usual please re-test it before pushing. Dave