public inbox for gcc-bugs@sourceware.org help / color / mirror / Atom feed
* [Bug jit/100688] New: Add support for link section @ 2021-05-19 23:42 bouanto at zoho dot com 2021-05-20 0:24 ` [Bug jit/100688] " bouanto at zoho dot com ` (9 more replies) 0 siblings, 10 replies; 11+ messages in thread From: bouanto at zoho dot com @ 2021-05-19 23:42 UTC (permalink / raw) To: gcc-bugs https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100688 Bug ID: 100688 Summary: Add support for link section Product: gcc Version: 10.3.1 Status: UNCONFIRMED Severity: normal Priority: P3 Component: jit Assignee: dmalcolm at gcc dot gnu.org Reporter: bouanto at zoho dot com Target Milestone: --- Hi. I'd like support to set the link section (i.e. `__attribute__((section(".section")))`) in libgccjit. A patch will follow soon. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug jit/100688] Add support for link section 2021-05-19 23:42 [Bug jit/100688] New: Add support for link section bouanto at zoho dot com @ 2021-05-20 0:24 ` bouanto at zoho dot com 2021-05-20 1:28 ` pinskia at gcc dot gnu.org ` (8 subsequent siblings) 9 siblings, 0 replies; 11+ messages in thread From: bouanto at zoho dot com @ 2021-05-20 0:24 UTC (permalink / raw) To: gcc-bugs https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100688 --- Comment #1 from Antoni <bouanto at zoho dot com> --- Created attachment 50847 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50847&action=edit Patch adding support for setting the link section ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug jit/100688] Add support for link section 2021-05-19 23:42 [Bug jit/100688] New: Add support for link section bouanto at zoho dot com 2021-05-20 0:24 ` [Bug jit/100688] " bouanto at zoho dot com @ 2021-05-20 1:28 ` pinskia at gcc dot gnu.org 2021-05-20 2:11 ` bouanto at zoho dot com ` (7 subsequent siblings) 9 siblings, 0 replies; 11+ messages in thread From: pinskia at gcc dot gnu.org @ 2021-05-20 1:28 UTC (permalink / raw) To: gcc-bugs https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100688 --- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> --- I am curious what is the use case here? ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug jit/100688] Add support for link section 2021-05-19 23:42 [Bug jit/100688] New: Add support for link section bouanto at zoho dot com 2021-05-20 0:24 ` [Bug jit/100688] " bouanto at zoho dot com 2021-05-20 1:28 ` pinskia at gcc dot gnu.org @ 2021-05-20 2:11 ` bouanto at zoho dot com 2021-05-20 2:41 ` pinskia at gcc dot gnu.org ` (6 subsequent siblings) 9 siblings, 0 replies; 11+ messages in thread From: bouanto at zoho dot com @ 2021-05-20 2:11 UTC (permalink / raw) To: gcc-bugs https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100688 --- Comment #3 from Antoni <bouanto at zoho dot com> --- I develop a gcc codegen for the Rust compiler and it's a feature of Rust to be able to set the link section: https://github.com/antoyo/rustc_codegen_gcc/commit/999f768526d72e19e3eafdc963dcb6af8a1afe60#diff-6bbb01450b42befd6551add5fd3f53dd6a0ebd54f6f32c20cef02246521bd490R19 One of the use cases is in the standard library: https://github.com/rust-lang/rust/blob/7e11f3a8f3c1b2683125e7def0acb68a6d684f92/library/std/src/sys/unix/args.rs#L112 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug jit/100688] Add support for link section 2021-05-19 23:42 [Bug jit/100688] New: Add support for link section bouanto at zoho dot com ` (2 preceding siblings ...) 2021-05-20 2:11 ` bouanto at zoho dot com @ 2021-05-20 2:41 ` pinskia at gcc dot gnu.org 2021-05-20 11:56 ` bouanto at zoho dot com ` (5 subsequent siblings) 9 siblings, 0 replies; 11+ messages in thread From: pinskia at gcc dot gnu.org @ 2021-05-20 2:41 UTC (permalink / raw) To: gcc-bugs https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100688 --- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> --- Hmm, now I am curious why you are using the jit framework rather than doing a normal GCC front-end; like was done for the other rust gcc front-end https://github.com/Rust-GCC/gccrs . ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug jit/100688] Add support for link section 2021-05-19 23:42 [Bug jit/100688] New: Add support for link section bouanto at zoho dot com ` (3 preceding siblings ...) 2021-05-20 2:41 ` pinskia at gcc dot gnu.org @ 2021-05-20 11:56 ` bouanto at zoho dot com 2021-05-20 13:32 ` dmalcolm at gcc dot gnu.org ` (4 subsequent siblings) 9 siblings, 0 replies; 11+ messages in thread From: bouanto at zoho dot com @ 2021-05-20 11:56 UTC (permalink / raw) To: gcc-bugs https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100688 --- Comment #5 from Antoni <bouanto at zoho dot com> --- This is much less work as I'm reusing the rustc front-end. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug jit/100688] Add support for link section 2021-05-19 23:42 [Bug jit/100688] New: Add support for link section bouanto at zoho dot com ` (4 preceding siblings ...) 2021-05-20 11:56 ` bouanto at zoho dot com @ 2021-05-20 13:32 ` dmalcolm at gcc dot gnu.org 2021-05-20 13:42 ` dmalcolm at gcc dot gnu.org ` (3 subsequent siblings) 9 siblings, 0 replies; 11+ messages in thread From: dmalcolm at gcc dot gnu.org @ 2021-05-20 13:32 UTC (permalink / raw) To: gcc-bugs https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100688 --- Comment #6 from David Malcolm <dmalcolm at gcc dot gnu.org> --- Thanks for the patch; I like the idea; various nits below: > diff --git a/gcc/jit/docs/topics/expressions.rst b/gcc/jit/docs/topics/expressions.rst > index 396259ef07e..b39f6c02527 100644 > --- a/gcc/jit/docs/topics/expressions.rst > +++ b/gcc/jit/docs/topics/expressions.rst > @@ -539,6 +539,18 @@ where the rvalue is computed by reading from the storage area. > > in C. > > +.. function:: void > + gcc_jit_lvalue_set_link_section (gcc_jit_lvalue *lvalue, > + const char *name) > + > + Set the link section of a variable; analogous to: > + > + .. code-block:: c > + > + int variable __attribute__((section(".section"))); > + > + in C. Please rename param "name" to "section_name". Your implementation requires that it be non-NULL (rather than having NULL unset the section), so please specify that it must be non-NULL in the docs. Please add the usual "This entrypoint was added in" text to state which API version it was added in. > + > Global variables > **************** > > diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h > index 825a3e172e9..8b0f65e87e8 100644 > --- a/gcc/jit/jit-playback.h > +++ b/gcc/jit/jit-playback.h > @@ -650,6 +650,8 @@ public: > > private: > context *m_ctxt; > + > +protected: > tree m_inner; > }; I think you only use this... > > @@ -670,6 +672,12 @@ public: > rvalue * > get_address (location *loc); > > + void > + set_link_section (const char* name) > + { > + set_decl_section_name (m_inner, name); > + } ...here, and you can get at rvalue::m_inner using as_tree (), so I don't think we need to make m_inner protected. > diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c > index 117ff70114c..d54f878cc6b 100644 > --- a/gcc/jit/jit-recording.c > +++ b/gcc/jit/jit-recording.c > @@ -3713,6 +3713,11 @@ recording::lvalue::get_address (recording::location *loc) > return result; > } > > +void recording::lvalue::set_link_section (const char *name) > +{ > + m_link_section = new_string (name); > +} > + > /* The implementation of class gcc::jit::recording::param. */ > > /* Implementation of pure virtual hook recording::memento::replay_into > @@ -4547,8 +4552,7 @@ recording::block::dump_edges_to_dot (pretty_printer *pp) > void > recording::global::replay_into (replayer *r) > { > - set_playback_obj ( > - m_initializer > + playback::lvalue *global = m_initializer > ? r->new_global_initialized (playback_location (r, m_loc), > m_kind, > m_type->playback_type (), > @@ -4560,7 +4564,12 @@ recording::global::replay_into (replayer *r) > : r->new_global (playback_location (r, m_loc), > m_kind, > m_type->playback_type (), > - playback_string (m_name))); > + playback_string (m_name)); > + if (m_link_section != NULL) > + { > + global->set_link_section(m_link_section->c_str()); > + } Coding convention nits: don't use {} when it's just one statement (which I think is a bad convention, but it is the project's convention). Missing spaces between function name and open-paren in both calls here. > + set_playback_obj (global); > } > [...snip....] > diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h > index 03fa1160cf0..0691fac579d 100644 > --- a/gcc/jit/jit-recording.h > +++ b/gcc/jit/jit-recording.h > @@ -1105,7 +1105,8 @@ public: > lvalue (context *ctxt, > location *loc, > type *type_) > - : rvalue (ctxt, loc, type_) > + : rvalue (ctxt, loc, type_), > + m_link_section(NULL) > {} > > playback::lvalue * > @@ -1127,6 +1128,10 @@ public: > const char *access_as_rvalue (reproducer &r) OVERRIDE; > virtual const char *access_as_lvalue (reproducer &r); > virtual bool is_global () const { return false; } > + void set_link_section (const char *name); > + > +protected: > + string *m_link_section; > }; Can it be private, rather than protected? > diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c > index 7fa948007ad..8cfa48aae24 100644 > --- a/gcc/jit/libgccjit.c > +++ b/gcc/jit/libgccjit.c > @@ -1953,6 +1953,18 @@ gcc_jit_lvalue_get_address (gcc_jit_lvalue *lvalue, > return (gcc_jit_rvalue *)lvalue->get_address (loc); > } > > +/* Public entrypoint. See description in libgccjit.h. > + > + After error-checking, the real work is done by the > + gcc::jit::recording::lvalue::set_section method in jit-recording.c. */ ^^^^^^^^^^^ set_link_section Also, a newline here please for consistency with the other entrypoints. > +void > +gcc_jit_lvalue_set_link_section (gcc_jit_lvalue *lvalue, > + const char *name) > +{ > + RETURN_IF_FAIL (name, NULL, NULL, "NULL name"); > + lvalue->set_link_section(name); Missing a space between function name and open-paren. > diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h > index 5c722c2c57f..21553ede3de 100644 > --- a/gcc/jit/libgccjit.h > +++ b/gcc/jit/libgccjit.h > @@ -1072,6 +1072,19 @@ extern gcc_jit_rvalue * > gcc_jit_lvalue_get_address (gcc_jit_lvalue *lvalue, > gcc_jit_location *loc); > > +#define LIBGCCJIT_HAVE_gcc_jit_lvalue_set_link_section > + > +/* Set the link section of a global variable; analogous to: > + __attribute__((section("section_name"))) > + in C. > + > + This API entrypoint was added in LIBGCCJIT_ABI_18; you can test for its > + presence using > + #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_tls_model */ Wrong #ifdef in the comment. > +extern void > +gcc_jit_lvalue_set_link_section (gcc_jit_lvalue *lvalue, > + const char *name); Rename param "name" to "section_name" to match the comment. > diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map > index 337ea6c7fe4..9e722c2bde1 100644 > --- a/gcc/jit/libgccjit.map > +++ b/gcc/jit/libgccjit.map > @@ -205,3 +205,14 @@ LIBGCCJIT_ABI_15 { > gcc_jit_extended_asm_add_clobber; > gcc_jit_context_add_top_level_asm; > } LIBGCCJIT_ABI_14; > + > +LIBGCCJIT_ABI_16 { > +} LIBGCCJIT_ABI_15; > + > +LIBGCCJIT_ABI_17 { > +} LIBGCCJIT_ABI_16; > + > +LIBGCCJIT_ABI_18 { > + global: > + gcc_jit_lvalue_set_link_section; > +} LIBGCCJIT_ABI_17; I have some other patches of yours to review (presumably where the other ABI things are); sorry about that. I'll try to get to them today. > diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h > index 4202eb7798b..7e3b59dee0d 100644 > --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h > +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h > @@ -181,6 +181,13 @@ > #undef create_code > #undef verify_code > > +/* test-link-section.c */ > +#define create_code create_code_link_section > +#define verify_code verify_code_link_section > +#include "test-link-section.c" > +#undef create_code > +#undef verify_code > + > /* test-hello-world.c */ > #define create_code create_code_hello_world > #define verify_code verify_code_hello_world > diff --git a/gcc/testsuite/jit.dg/test-link-section.c b/gcc/testsuite/jit.dg/test-link-section.c > new file mode 100644 > index 00000000000..546c1e95b92 > --- /dev/null > +++ b/gcc/testsuite/jit.dg/test-link-section.c > @@ -0,0 +1,25 @@ > +#include <stdlib.h> > +#include <stdio.h> > + > +#include "libgccjit.h" > + > +#include "harness.h" > + > +void > +create_code (gcc_jit_context *ctxt, void *user_data) > +{ > + /* Let's try to inject the equivalent of: > + int foo __attribute__((section(".section"))); > + */ > + 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_link_section(foo, "section"); > +} > + > +extern void > +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result) > +{ > +} This is OK, but ideally it would test that the section name made it into the generated assembler. test-compile-to-assembler.c has a testcase for this which does something similar, with a DejaGnu directive looking for a substring in the generated asm if you want to attempt it. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug jit/100688] Add support for link section 2021-05-19 23:42 [Bug jit/100688] New: Add support for link section bouanto at zoho dot com ` (5 preceding siblings ...) 2021-05-20 13:32 ` dmalcolm at gcc dot gnu.org @ 2021-05-20 13:42 ` dmalcolm at gcc dot gnu.org 2021-11-20 0:32 ` dmalcolm at gcc dot gnu.org ` (2 subsequent siblings) 9 siblings, 0 replies; 11+ messages in thread From: dmalcolm at gcc dot gnu.org @ 2021-05-20 13:42 UTC (permalink / raw) To: gcc-bugs https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100688 --- Comment #7 from David Malcolm <dmalcolm at gcc dot gnu.org> --- One other thing: the docs should make it clear about the leading ".". If I want to create the equivalent of: __attribute__((section(".section"))) do I call it with: gcc_jit_lvalue_set_link_section(foo, "section"); or with: gcc_jit_lvalue_set_link_section(foo, ".section"); It's a bit unclear to me from just reading the patch. The example suggests it's the former. In either case, the documentation should be clearer about this. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug jit/100688] Add support for link section 2021-05-19 23:42 [Bug jit/100688] New: Add support for link section bouanto at zoho dot com ` (6 preceding siblings ...) 2021-05-20 13:42 ` dmalcolm at gcc dot gnu.org @ 2021-11-20 0:32 ` dmalcolm at gcc dot gnu.org 2021-12-12 21:17 ` cvs-commit at gcc dot gnu.org 2021-12-13 0:37 ` bouanto at zoho dot com 9 siblings, 0 replies; 11+ messages in thread From: dmalcolm at gcc dot gnu.org @ 2021-11-20 0:32 UTC (permalink / raw) To: gcc-bugs https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100688 David Malcolm <dmalcolm at gcc dot gnu.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |WAITING Ever confirmed|0 |1 Last reconfirmed| |2021-11-20 --- Comment #8 from David Malcolm <dmalcolm at gcc dot gnu.org> --- Thanks for working on this. I believe the current status is that I found some nits in review as above, along with: https://gcc.gnu.org/pipermail/jit/2021q2/001305.html so the current status is: waiting on an updated patch. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug jit/100688] Add support for link section 2021-05-19 23:42 [Bug jit/100688] New: Add support for link section bouanto at zoho dot com ` (7 preceding siblings ...) 2021-11-20 0:32 ` dmalcolm at gcc dot gnu.org @ 2021-12-12 21:17 ` cvs-commit at gcc dot gnu.org 2021-12-13 0:37 ` bouanto at zoho dot com 9 siblings, 0 replies; 11+ messages in thread From: cvs-commit at gcc dot gnu.org @ 2021-12-12 21:17 UTC (permalink / raw) To: gcc-bugs https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100688 --- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> --- The master branch has been updated by Antoni Boucher <antoyo@gcc.gnu.org>: https://gcc.gnu.org/g:0b52083ea2c2dd9897031fdc3802a68fd4aa45ef commit r12-5917-g0b52083ea2c2dd9897031fdc3802a68fd4aa45ef Author: Antoni Boucher <bouanto@zoho.com> Date: Sun Dec 12 16:16:21 2021 -0500 libgccjit: Add support for setting the link section of global variables [PR100688] 2021-12-12 Antoni Boucher <bouanto@zoho.com> gcc/jit/ PR target/100688 * docs/topics/compatibility.rst (LIBGCCJIT_ABI_18): New ABI tag. * docs/topics/expressions.rst: Add documentation for the function gcc_jit_lvalue_set_link_section. * jit-playback.h: New function (set_link_section). * jit-recording.c: New function (set_link_section) and support for setting the link section. * jit-recording.h: New function (set_link_section) and new field m_link_section. * libgccjit.c: New function (gcc_jit_lvalue_set_link_section). * libgccjit.h: New function (gcc_jit_lvalue_set_link_section). * libgccjit.map (LIBGCCJIT_ABI_18): New ABI tag. gcc/testsuite/ PR target/100688 * jit.dg/all-non-failing-tests.h: Mention new test link-section-assembler. * jit.dg/test-link-section-assembler.c: New test. * jit.dg/jit.exp: New helper function to test that the assembly contains a pattern. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug jit/100688] Add support for link section 2021-05-19 23:42 [Bug jit/100688] New: Add support for link section bouanto at zoho dot com ` (8 preceding siblings ...) 2021-12-12 21:17 ` cvs-commit at gcc dot gnu.org @ 2021-12-13 0:37 ` bouanto at zoho dot com 9 siblings, 0 replies; 11+ messages in thread From: bouanto at zoho dot com @ 2021-12-13 0:37 UTC (permalink / raw) To: gcc-bugs https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100688 Antoni <bouanto at zoho dot com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|WAITING |RESOLVED Resolution|--- |FIXED --- Comment #10 from Antoni <bouanto at zoho dot com> --- Fixed in master. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-12-13 0:37 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-19 23:42 [Bug jit/100688] New: Add support for link section bouanto at zoho dot com 2021-05-20 0:24 ` [Bug jit/100688] " bouanto at zoho dot com 2021-05-20 1:28 ` pinskia at gcc dot gnu.org 2021-05-20 2:11 ` bouanto at zoho dot com 2021-05-20 2:41 ` pinskia at gcc dot gnu.org 2021-05-20 11:56 ` bouanto at zoho dot com 2021-05-20 13:32 ` dmalcolm at gcc dot gnu.org 2021-05-20 13:42 ` dmalcolm at gcc dot gnu.org 2021-11-20 0:32 ` dmalcolm at gcc dot gnu.org 2021-12-12 21:17 ` cvs-commit at gcc dot gnu.org 2021-12-13 0:37 ` bouanto at zoho dot com
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).