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).