From: Antoni Boucher <bouanto@zoho.com>
To: David Malcolm <dmalcolm@redhat.com>,
jit@gcc.gnu.org, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] libgccjit: Add support for setting the link section of global variables [PR100688]
Date: Sat, 20 Nov 2021 11:53:07 -0500 [thread overview]
Message-ID: <ea65c093c81f559232593291a7cda4c2fa83dd79.camel@zoho.com> (raw)
In-Reply-To: <7530c6bca7aab2a29303957512be97cf090e159d.camel@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 14370 bytes --]
Hi.
Here's the updated patch.
See comments below.
Thanks for the review!
Le samedi 20 novembre 2021 à 11:20 -0500, David Malcolm a écrit :
> On Sat, 2021-11-20 at 00:58 -0500, Antoni Boucher wrote:
> > Thanks for your reviews!
> >
> > Here's the updated patch, ready for another review.
> > See comments/questions below.
>
> Thanks for the updated patch...
>
> >
> > I'll update the other patches over the weekend.
> >
> > Le jeudi 20 mai 2021 à 15:29 -0400, David Malcolm a écrit :
> > > On Wed, 2021-05-19 at 20:32 -0400, Antoni Boucher via Jit wrote:
> > > > Hello.
> > > > This patch adds support to set the link section of global
> > > > variables.
> > > > I used the ABI 18 because I submitted other patches up to 17.
> > > > Thanks for the review.
> > >
> > > I didn't see this email until now, and put the review in bugzilla
> > > instead; sorry.
> > >
> > > Here's a copy-and-paste of what I put in bugzilla:
> > >
> > >
> > > Thanks for the patch; I like the idea; various nits below:
> > >
>
> [...snip...]
>
> > >
> > > > 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.
>
> Thanks for dropping the "protected" in the updated patch; you need to
> update the ChangeLog entry.
>
> > >
> > >
> > > > + 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?
> >
> > m_link_section can't be private because it's used in
> > recording::global::replay_into.
>
> Fair enough; thanks.
>
> > >
> > > > 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.
> >
> > Where should I add a newline?
>
> Between the closing of the comment and the "void" of the fndecl (all
> the other entrypoints have a blank line separating them). Thanks for
> fixing my other nitpicks.
>
>
> [...snip...]
>
> > > > 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
>
> The updated version of the testcase doesn't have a verify_code, so it
> can't be in the "testcases" array.
> Please replace this with something like:
>
>
> /* test-link-section.c: This can't be in the testcases array as it
> doesn't have a verify_code implementation. */
>
>
> (I'm trying to have all-nonfailing-tests.h refer to each non-failing
> test, either adding it to the testcases array, or documenting why it
> isn't in the array; listing them in alphabetical order)
Hum, that code seems to refer to the previous patch.
I removed that part and changed the test to test-link-section-
assembler.c which checks that the assembly contains the link section.
As far as I know, it does not need to be listed here to be ran.
>
> [...snip...]
>
> > > >
> > > > +
> > > > +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.
>
> Thanks for implementing the .exp directive.
>
> > >
> > > 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.
>
> Thanks for clarifying this in the docs.
> >
>
> Notes on the updated patch follow:
>
>
> > From a454cb9ce14aa6903f7be9f2a56c35ab8251e678 Mon Sep 17 00:00:00
> > 2001
> > From: Antoni Boucher <bouanto@zoho.com>
> > Date: Wed, 12 May 2021 07:57:54 -0400
> > Subject: [PATCH] libgccjit: Add support for setting the link
> > section of global
> > variables [PR100688]
> >
> > 2021-11-20 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) and
> > rvalue::m_inner protected.
>
> As noted above, this part of the ChangeLog at least needs updating.
>
> > * 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: Add test-link-
> > section.c.
> > * jit.dg/test-link_section.c: New test.
> > * jit.dg/jit.exp: New helper function to test that the
> > assembly contains a pattern.
>
> [...snip...]
>
> > diff --git a/gcc/jit/docs/topics/compatibility.rst
> > b/gcc/jit/docs/topics/compatibility.rst
> > index 52ee3f860a7..b852f205fd7 100644
> > --- a/gcc/jit/docs/topics/compatibility.rst
> > +++ b/gcc/jit/docs/topics/compatibility.rst
> > @@ -284,3 +284,12 @@ entrypoints:
> > * :func:`gcc_jit_struct_get_field`
> >
> > * :func:`gcc_jit_struct_get_field_count`
> > +
> > +.. _LIBGCCJIT_ABI_18:
> > +
> > +``LIBGCCJIT_ABI_18``
> > +-----------------------
> > +``LIBGCCJIT_ABI_18`` covers the addition of an API entrypoint to
> > set the link
> > +section of a variable:
> > +
> > + * :func:`gcc_jit_lvalue_set_link_section`
>
> As noted in earlier review, presumably there's a different patch that
> adds ABI_17?
>
> I can potentially approve this link section patch now, but that other
> patch would need to be pushed first.
Yes, there's a patch before this one that will need to be reviewed and
merged first.
>
> [...snip...]
>
> > @@ -4547,20 +4552,30 @@ recording::block::dump_edges_to_dot
> > (pretty_printer *pp)
> > void
> > recording::global::replay_into (replayer *r)
> > {
> > - set_playback_obj (
> > - m_initializer
> > - ? r->new_global_initialized (playback_location (r, m_loc),
> > + playback::lvalue *global;
> > + if (m_initializer)
> > + {
> > + global = r->new_global_initialized (playback_location (r,
> > m_loc),
> > m_kind,
> > m_type->playback_type (),
> > m_type->dereference ()->get_size
> > (),
> > m_initializer_num_bytes
> > / m_type->dereference ()->get_size
> > (),
> > m_initializer,
> > - playback_string (m_name))
> > - : r->new_global (playback_location (r, m_loc),
> > + playback_string (m_name));
> > + }
> > + else
> > + {
> > + global = 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 ());
> > +
> > + set_playback_obj (global);
> > }
>
> Looks like the patch as posted has some interaction with the
> initializers patch, presumably that's the ABI_17?
Yes, but that's gonna be in a future patch that is not ABI 17. Since I
need to update that code to have the variable global to call
set_link_section, I'll let this code as is if this is OK.
>
> But presumably the intent here is this hunk:
>
> + if (m_link_section != NULL)
> + global->set_link_section (m_link_section->c_str ());
>
> and that indeed looks good.
>
>
> > diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
> > index c744b634f4b..5b413d41e2c 100644
> > --- a/gcc/jit/libgccjit.c
> > +++ b/gcc/jit/libgccjit.c
> > @@ -2217,6 +2217,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_link_section method in jit-
> > recording.c. */
> > +void
> > +gcc_jit_lvalue_set_link_section (gcc_jit_lvalue *lvalue,
> > + const char *section_name)
> > +{
> > + RETURN_IF_FAIL (name, NULL, NULL, "NULL section_name");
> > + lvalue->set_link_section (name);
> > +}
>
> Thanks for renaming the param from "name" to "section_name", but it
> looks like you didn't rename the uses of it within the function - I'm
> guessing this code as-is doesn't compile.
>
> [...snip...]
>
> > diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
> > index 64e790949e8..e5bdafd0156 100644
> > --- a/gcc/jit/libgccjit.map
> > +++ b/gcc/jit/libgccjit.map
> > @@ -226,3 +226,11 @@ LIBGCCJIT_ABI_16 {
> > gcc_jit_type_is_struct;
> > gcc_jit_struct_get_field_count;
> > } LIBGCCJIT_ABI_15;
> > +
> > +LIBGCCJIT_ABI_17 {
> > +} LIBGCCJIT_ABI_16;
> > +
>
> Same notes here as above.
>
> [...snip...]
>
> Overall the patch looks good, my comments are all just nit-picks at
> this point, except that obviously you need to finish updating the
> symbol name to "section_name" in gcc_jit_lvalue_set_link_section so
> that it compiles. Please check it compiles and that the testsuite
> passes before pushing - though I think this is waiting on another
> patch, right? This is OK for trunk, once those fixes and
> prerequisites
> are taken care of.
>
> Thanks again
> Dave
>
[-- Attachment #2: 0001-libgccjit-Add-support-for-setting-the-link-section-o.patch --]
[-- Type: text/x-patch, Size: 11731 bytes --]
From d334fbaca8fa40bebcef88cba2f05329d677c69c Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Wed, 12 May 2021 07:57:54 -0400
Subject: [PATCH] libgccjit: Add support for setting the link section of global
variables [PR100688]
2021-11-20 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/test-link-section-assembler.c: New test.
* jit.dg/jit.exp: New helper function to test that the
assembly contains a pattern.
---
gcc/jit/docs/topics/compatibility.rst | 9 +++++
gcc/jit/docs/topics/expressions.rst | 21 +++++++++++
gcc/jit/jit-playback.h | 6 +++
gcc/jit/jit-recording.c | 35 +++++++++++++++---
gcc/jit/jit-recording.h | 7 +++-
gcc/jit/libgccjit.c | 13 +++++++
gcc/jit/libgccjit.h | 14 +++++++
gcc/jit/libgccjit.map | 8 ++++
gcc/testsuite/jit.dg/jit.exp | 33 +++++++++++++++++
.../jit.dg/test-link-section-assembler.c | 37 +++++++++++++++++++
10 files changed, 176 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/jit.dg/test-link-section-assembler.c
diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
index 52ee3f860a7..b852f205fd7 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -284,3 +284,12 @@ entrypoints:
* :func:`gcc_jit_struct_get_field`
* :func:`gcc_jit_struct_get_field_count`
+
+.. _LIBGCCJIT_ABI_18:
+
+``LIBGCCJIT_ABI_18``
+-----------------------
+``LIBGCCJIT_ABI_18`` covers the addition of an API entrypoint to set the link
+section of a variable:
+
+ * :func:`gcc_jit_lvalue_set_link_section`
diff --git a/gcc/jit/docs/topics/expressions.rst b/gcc/jit/docs/topics/expressions.rst
index 396259ef07e..02de531f31a 100644
--- a/gcc/jit/docs/topics/expressions.rst
+++ b/gcc/jit/docs/topics/expressions.rst
@@ -539,6 +539,27 @@ 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 *section_name)
+
+ Set the link section of a variable.
+ The parameter ``section_name`` must be non-NULL and must contains the
+ leading dot. Analogous to:
+
+ .. code-block:: c
+
+ int variable __attribute__((section(".section")));
+
+ in C.
+
+ This entrypoint was added in :ref:`LIBGCCJIT_ABI_18`; you can test for
+ its presence using
+
+ .. code-block:: c
+
+ #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_link_section
+
Global variables
****************
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index f670c9e81df..15ff4f91eb7 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -675,6 +675,12 @@ public:
rvalue *
get_address (location *loc);
+ void
+ set_link_section (const char* name)
+ {
+ set_decl_section_name (as_tree (), name);
+ }
+
private:
bool mark_addressable (location *loc);
};
diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
index 117ff70114c..2ad9e90e397 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,20 +4552,30 @@ recording::block::dump_edges_to_dot (pretty_printer *pp)
void
recording::global::replay_into (replayer *r)
{
- set_playback_obj (
- m_initializer
- ? r->new_global_initialized (playback_location (r, m_loc),
+ playback::lvalue *global;
+ if (m_initializer)
+ {
+ global = r->new_global_initialized (playback_location (r, m_loc),
m_kind,
m_type->playback_type (),
m_type->dereference ()->get_size (),
m_initializer_num_bytes
/ m_type->dereference ()->get_size (),
m_initializer,
- playback_string (m_name))
- : r->new_global (playback_location (r, m_loc),
+ playback_string (m_name));
+ }
+ else
+ {
+ global = 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 ());
+
+ set_playback_obj (global);
}
/* Override the default implementation of
@@ -4675,6 +4690,14 @@ recording::global::write_reproducer (reproducer &r)
r.get_identifier_as_type (get_type ()),
m_name->get_debug_string ());
+ if (m_link_section != NULL)
+ {
+ r.write (" gcc_jit_lvalue_set_link_section (%s, /* gcc_jit_lvalue *lvalue */\n"
+ " \"%s\"); /* */\n",
+ id,
+ m_link_section->c_str ());
+ }
+
if (m_initializer)
switch (m_type->dereference ()->get_size ())
{
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 4a994fe7094..2d08e043e10 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -1112,7 +1112,8 @@ public:
lvalue (context *ctxt,
location *loc,
type *type_)
- : rvalue (ctxt, loc, type_)
+ : rvalue (ctxt, loc, type_),
+ m_link_section (NULL)
{}
playback::lvalue *
@@ -1134,6 +1135,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;
};
class param : public lvalue
diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
index c744b634f4b..7da1fab7438 100644
--- a/gcc/jit/libgccjit.c
+++ b/gcc/jit/libgccjit.c
@@ -2217,6 +2217,19 @@ 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_link_section method in jit-recording.c. */
+
+void
+gcc_jit_lvalue_set_link_section (gcc_jit_lvalue *lvalue,
+ const char *section_name)
+{
+ RETURN_IF_FAIL (section_name, NULL, NULL, "NULL section_name");
+ lvalue->set_link_section (section_name);
+}
+
/* Public entrypoint. See description in libgccjit.h.
After error-checking, the real work is done by the
diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index a1c9436c545..646b5172b08 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -1089,6 +1089,20 @@ 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_link_section
+*/
+extern void
+gcc_jit_lvalue_set_link_section (gcc_jit_lvalue *lvalue,
+ const char *section_name);
+
extern gcc_jit_lvalue *
gcc_jit_function_new_local (gcc_jit_function *func,
gcc_jit_location *loc,
diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
index 64e790949e8..e5bdafd0156 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -226,3 +226,11 @@ LIBGCCJIT_ABI_16 {
gcc_jit_type_is_struct;
gcc_jit_struct_get_field_count;
} LIBGCCJIT_ABI_15;
+
+LIBGCCJIT_ABI_17 {
+} LIBGCCJIT_ABI_16;
+
+LIBGCCJIT_ABI_18 {
+ global:
+ gcc_jit_lvalue_set_link_section;
+} LIBGCCJIT_ABI_17;
diff --git a/gcc/testsuite/jit.dg/jit.exp b/gcc/testsuite/jit.dg/jit.exp
index 10b98bdc74b..3568dbb9d63 100644
--- a/gcc/testsuite/jit.dg/jit.exp
+++ b/gcc/testsuite/jit.dg/jit.exp
@@ -864,6 +864,39 @@ proc jit-verify-assembler { args } {
jit-run-executable ${executable_from_asm} ${dg-output-text}
}
+# Assuming that a .s file has been written out named
+# OUTPUT_FILENAME, check that the argument matches the
+# output file.
+# For use by the test-link-section-assembler.c testcase.
+proc jit-verify-assembler-output { args } {
+ verbose "jit-verify-assembler: $args"
+
+ set dg-output-text [lindex $args 0]
+ verbose "dg-output-text: ${dg-output-text}"
+
+ upvar 2 name name
+ verbose "name: $name"
+
+ upvar 2 prog prog
+ verbose "prog: $prog"
+ set asm_filename [jit-get-output-filename $prog]
+ verbose " asm_filename: ${asm_filename}"
+
+ # Read the assembly file.
+ set f [open $asm_filename r]
+ set content [read $f]
+ close $f
+
+ # Verify that the assembly matches the regex.
+ if { ![regexp ${dg-output-text} $content] } {
+ fail "${asm_filename} output pattern test, is ${content}, should match ${dg-output-text}"
+ verbose "Failed test for output pattern ${dg-output-text}" 3
+ } else {
+ pass "${asm_filename} output pattern test, ${dg-output-text}"
+ verbose "Passed test for output pattern ${dg-output-text}" 3
+ }
+
+}
# Assuming that a .o file has been written out named
# OUTPUT_FILENAME, invoke the driver to try to turn it into
# an executable, and try to run the result.
diff --git a/gcc/testsuite/jit.dg/test-link-section-assembler.c b/gcc/testsuite/jit.dg/test-link-section-assembler.c
new file mode 100644
index 00000000000..a90b00e9a82
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-link-section-assembler.c
@@ -0,0 +1,37 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "libgccjit.h"
+
+#define TEST_COMPILING_TO_FILE
+#define OUTPUT_KIND GCC_JIT_OUTPUT_KIND_ASSEMBLER
+#define OUTPUT_FILENAME "output-of-test-link-section-assembler.c.s"
+#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, ".my_section");
+
+ gcc_jit_function *func_main =
+ gcc_jit_context_new_function (ctxt, NULL,
+ GCC_JIT_FUNCTION_EXPORTED,
+ int_type,
+ "main",
+ 0, NULL,
+ 0);
+ gcc_jit_rvalue *zero = gcc_jit_context_zero (ctxt, int_type);
+ gcc_jit_block *block = gcc_jit_function_new_block (func_main, NULL);
+ gcc_jit_block_end_with_return (block, NULL, zero);
+}
+
+/* { dg-final { jit-verify-output-file-was-created "" } } */
+/* { dg-final { jit-verify-assembler-output ".section .my_section" } } */
--
2.26.2.7.g19db9cfb68.dirty
next prev parent reply other threads:[~2021-11-20 16:53 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-20 0:32 Antoni Boucher
2021-05-20 19:29 ` David Malcolm
2021-05-20 19:59 ` David Malcolm
2021-11-20 5:58 ` Antoni Boucher
2021-11-20 16:20 ` David Malcolm
2021-11-20 16:53 ` Antoni Boucher [this message]
2021-11-20 18:27 ` David Malcolm
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ea65c093c81f559232593291a7cda4c2fa83dd79.camel@zoho.com \
--to=bouanto@zoho.com \
--cc=dmalcolm@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jit@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).