[PATCH] libgccjit: Add support for setting the alignment [PR104293]

[PATCH] libgccjit: Add support for setting the alignment [PR104293]

[Antoni Boucher]

[-- Attachment #1: Type: text/plain, Size: 211 bytes --]

Hi.
This patch adds support for setting the alignment of variables in
libgccjit.

I was wondering if I should change it so that it takes/returns bytes
instead of bits.
What do you think?

Thanks for the review.

[-- Attachment #2: 0001-libgccjit-Add-support-for-setting-the-alignment-PR10.patch --]
[-- Type: text/x-patch, Size: 11966 bytes --]

From ebdb6905f23ddef28292a1d71081eebb7a2a9bb9 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Tue, 25 Jan 2022 21:12:32 -0500
Subject: [PATCH] libgccjit: Add support for setting the alignment [PR104293]

2022-01-30  Antoni Boucher <bouanto@zoho.com>

gcc/jit/
	PR jit/104293
	* docs/topics/compatibility.rst (LIBGCCJIT_ABI_24): New ABI tag.
	* docs/topics/expressions.rst: Add documentation for the
	functions gcc_jit_lvalue_set_alignment and
	gcc_jit_lvalue_get_alignment.
	* jit-playback.h: New function (set_alignment).
	* jit-recording.cc: New function (set_alignment).
	* jit-recording.h: New functions (set_alignment, get_alignment)
	and new field (m_alignment).
	* libgccjit.cc: New functions (gcc_jit_lvalue_get_alignment,
	gcc_jit_lvalue_set_alignment)
	* libgccjit.h: New functions (gcc_jit_lvalue_get_alignment,
	gcc_jit_lvalue_set_alignment)
	* libgccjit.map (LIBGCCJIT_ABI_24): New ABI tag.

gcc/testsuite/
	PR jit/104293
	* jit.dg/all-non-failing-tests.h: Mention
	test-setting-alignment.
	* jit.dg/test-setting-alignment.c: New test.
---
 gcc/jit/docs/topics/compatibility.rst         | 10 +++
 gcc/jit/docs/topics/expressions.rst           | 39 +++++++++++
 gcc/jit/jit-playback.h                        |  7 ++
 gcc/jit/jit-recording.cc                      | 17 ++++-
 gcc/jit/jit-recording.h                       |  6 +-
 gcc/jit/libgccjit.cc                          | 25 ++++++++
 gcc/jit/libgccjit.h                           | 19 ++++++
 gcc/jit/libgccjit.map                         | 18 ++++++
 gcc/testsuite/jit.dg/all-non-failing-tests.h  |  3 +
 gcc/testsuite/jit.dg/test-setting-alignment.c | 64 +++++++++++++++++++
 10 files changed, 205 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/jit.dg/test-setting-alignment.c

diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
index 16cebe31a10..1957399dceb 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -302,3 +302,13 @@ thread-local storage model of a variable:
 section of a variable:
 
   * :func:`gcc_jit_lvalue_set_link_section`
+
+.. _LIBGCCJIT_ABI_24:
+
+``LIBGCCJIT_ABI_24``
+-----------------------
+``LIBGCCJIT_ABI_24`` covers the addition of functions to get and set the
+alignement of a variable:
+
+  * :func:`gcc_jit_lvalue_set_alignment`
+  * :func:`gcc_jit_lvalue_get_alignment`
diff --git a/gcc/jit/docs/topics/expressions.rst b/gcc/jit/docs/topics/expressions.rst
index 791a20398ca..0f5f5376d8c 100644
--- a/gcc/jit/docs/topics/expressions.rst
+++ b/gcc/jit/docs/topics/expressions.rst
@@ -738,6 +738,45 @@ where the rvalue is computed by reading from the storage area.
 
       #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_link_section
 
+.. function:: void
+              gcc_jit_lvalue_set_alignment (gcc_jit_lvalue *lvalue,
+                                            int alignment)
+
+   Set the alignment of a variable.
+   The parameter ``alignment`` is in bits. Analogous to:
+
+   .. code-block:: c
+
+     int variable __attribute__((aligned (16)));
+
+   in C, but in bits instead of bytes.
+
+   This entrypoint was added in :ref:`LIBGCCJIT_ABI_24`; you can test for
+   its presence using
+
+   .. code-block:: c
+
+      #ifdef LIBGCCJIT_HAVE_ALIGNMENT
+
+.. function:: int
+              gcc_jit_lvalue_get_alignment (gcc_jit_lvalue *lvalue)
+
+   Return the alignment of a variable set by ``gcc_jit_lvalue_set_alignment``, in bits.
+   Return 0 if the alignment was not set. Analogous to:
+
+   .. code-block:: c
+
+     _Alignof (variable)
+
+   in C, but in bits instead of bytes.
+
+   This entrypoint was added in :ref:`LIBGCCJIT_ABI_24`; you can test for
+   its presence using
+
+   .. code-block:: c
+
+      #ifdef LIBGCCJIT_HAVE_ALIGNMENT
+
 Global variables
 ****************
 
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index c93d7055d43..4a3c4a6a09b 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -701,6 +701,13 @@ public:
     set_decl_section_name (as_tree (), name);
   }
 
+  void
+  set_alignment (int alignment)
+  {
+      SET_DECL_ALIGN (as_tree (), alignment);
+      DECL_USER_ALIGN (as_tree ()) = 1;
+  }
+
 private:
   bool mark_addressable (location *loc);
 };
diff --git a/gcc/jit/jit-recording.cc b/gcc/jit/jit-recording.cc
index 1e3fadfacd7..7176fb34734 100644
--- a/gcc/jit/jit-recording.cc
+++ b/gcc/jit/jit-recording.cc
@@ -3807,6 +3807,11 @@ void recording::lvalue::set_link_section (const char *name)
   m_link_section = new_string (name);
 }
 
+void recording::lvalue::set_alignment (int alignment)
+{
+  m_alignment = alignment;
+}
+
 /* The implementation of class gcc::jit::recording::param.  */
 
 /* Implementation of pure virtual hook recording::memento::replay_into
@@ -4673,6 +4678,9 @@ recording::global::replay_into (replayer *r)
   if (m_link_section != NULL)
     global->set_link_section (m_link_section->c_str ());
 
+  if (m_alignment != 0)
+    global->set_alignment (m_alignment);
+
   set_playback_obj (global);
 }
 
@@ -6343,11 +6351,16 @@ recording::function_pointer::write_reproducer (reproducer &r)
 void
 recording::local::replay_into (replayer *r)
 {
-  set_playback_obj (
+  playback::lvalue *local =
     m_func->playback_function ()
       ->new_local (playback_location (r, m_loc),
 		   m_type->playback_type (),
-		   playback_string (m_name)));
+		   playback_string (m_name));
+
+  if (m_alignment != 0)
+    local->set_alignment (m_alignment);
+
+  set_playback_obj (local);
 }
 
 /* Override the default implementation of
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 846d65cb202..f0e936ccb30 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -1148,7 +1148,8 @@ public:
 	  type *type_)
     : rvalue (ctxt, loc, type_),
     m_tls_model (GCC_JIT_TLS_MODEL_NONE),
-    m_link_section (NULL)
+    m_link_section (NULL),
+    m_alignment (0)
     {}
 
   playback::lvalue *
@@ -1172,10 +1173,13 @@ public:
   virtual bool is_global () const { return false; }
   void set_tls_model (enum gcc_jit_tls_model model);
   void set_link_section (const char *name);
+  void set_alignment (int alignment);
+  int get_alignment () { return m_alignment; }
 
 protected:
   enum gcc_jit_tls_model m_tls_model;
   string *m_link_section;
+  int m_alignment;
 };
 
 class param : public lvalue
diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
index 4c352e8c93d..e03f15ec9c8 100644
--- a/gcc/jit/libgccjit.cc
+++ b/gcc/jit/libgccjit.cc
@@ -2649,6 +2649,31 @@ gcc_jit_lvalue_set_link_section (gcc_jit_lvalue *lvalue,
   lvalue->set_link_section (section_name);
 }
 
+/* Public entrypoint.  See description in libgccjit.h.
+
+   After error-checking, the real work is done by the
+   gcc::jit::recording::lvalue::get_link_section method in jit-recording.cc.  */
+
+int
+gcc_jit_lvalue_get_alignment (gcc_jit_lvalue *lvalue)
+{
+  RETURN_VAL_IF_FAIL (lvalue, 0, NULL, NULL, "NULL lvalue");
+  return lvalue->get_alignment ();
+}
+
+/* Public entrypoint.  See description in libgccjit.h.
+
+   After error-checking, the real work is done by the
+   gcc::jit::recording::lvalue::set_alignment method in jit-recording.cc.  */
+
+void
+gcc_jit_lvalue_set_alignment (gcc_jit_lvalue *lvalue,
+			      int alignment)
+{
+  RETURN_IF_FAIL (lvalue, NULL, NULL, "NULL lvalue");
+  lvalue->set_alignment (alignment);
+}
+
 /* 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 2a5ffacb1fe..74969871c06 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -1277,6 +1277,25 @@ extern void
 gcc_jit_lvalue_set_link_section (gcc_jit_lvalue *lvalue,
 			    const char *section_name);
 
+#define LIBGCCJIT_HAVE_ALIGNMENT
+
+/* Set the alignment of a variable.
+
+   This API entrypoint was added in LIBGCCJIT_ABI_24; you can test for its
+   presence using
+     #ifdef LIBGCCJIT_HAVE_ALIGNMENT  */
+extern void
+gcc_jit_lvalue_set_alignment (gcc_jit_lvalue *lvalue,
+			      int alignment);
+
+/* Get the alignment of a variable.
+
+   This API entrypoint was added in LIBGCCJIT_ABI_24; you can test for its
+   presence using
+     #ifdef LIBGCCJIT_HAVE_ALIGNMENT  */
+extern int
+gcc_jit_lvalue_get_alignment (gcc_jit_lvalue *lvalue);
+
 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 f373fd39ac7..df51e4fdd8c 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -243,3 +243,21 @@ LIBGCCJIT_ABI_19 {
     gcc_jit_context_new_union_constructor;
     gcc_jit_global_set_initializer_rvalue;
 } LIBGCCJIT_ABI_18;
+
+LIBGCCJIT_ABI_20 {
+} LIBGCCJIT_ABI_19;
+
+LIBGCCJIT_ABI_21 {
+} LIBGCCJIT_ABI_20;
+
+LIBGCCJIT_ABI_22 {
+} LIBGCCJIT_ABI_21;
+
+LIBGCCJIT_ABI_23 {
+} LIBGCCJIT_ABI_22;
+
+LIBGCCJIT_ABI_24 {
+  global:
+    gcc_jit_lvalue_set_alignment;
+    gcc_jit_lvalue_get_alignment;
+} LIBGCCJIT_ABI_23;
diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index 29afe064db6..72c46e81e51 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -306,6 +306,9 @@
 #undef create_code
 #undef verify_code
 
+/* test-setting-alignment.c: This can't be in the testcases array as it
+   doesn't have a verify_code implementation.  */
+
 /* test-string-literal.c */
 #define create_code create_code_string_literal
 #define verify_code verify_code_string_literal
diff --git a/gcc/testsuite/jit.dg/test-setting-alignment.c b/gcc/testsuite/jit.dg/test-setting-alignment.c
new file mode 100644
index 00000000000..e87afbeacd3
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-setting-alignment.c
@@ -0,0 +1,64 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "libgccjit.h"
+
+/* We don't want set_options() in harness.h to set -O3 so our little local
+   is optimized away. */
+#define TEST_ESCHEWS_SET_OPTIONS
+static void set_options (gcc_jit_context *ctxt, const char *argv0)
+{
+}
+
+#define TEST_COMPILING_TO_FILE
+#define OUTPUT_KIND      GCC_JIT_OUTPUT_KIND_ASSEMBLER
+#define OUTPUT_FILENAME  "output-of-test-setting-alignment.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__((aligned (8)));
+
+     int main (void) {
+        int bar __attribute__((aligned (16)));
+     }
+  */
+  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_alignment(foo, 64);
+
+  gcc_jit_field *field = gcc_jit_context_new_field (ctxt,
+    NULL,
+    int_type,
+    "a");
+  gcc_jit_struct *struct_type =
+    gcc_jit_context_new_struct_type(ctxt, NULL, "Type", 1, &field);
+  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_lvalue *bar =
+    gcc_jit_function_new_local (
+      func_main, NULL,
+      gcc_jit_struct_as_type (struct_type),
+      "bar");
+  gcc_jit_lvalue_set_alignment(bar, 128);
+  gcc_jit_block *block = gcc_jit_function_new_block (func_main, NULL);
+  /*gcc_jit_block_add_assignment (block, NULL, bar, zero);*/
+  gcc_jit_rvalue *return_value =
+      gcc_jit_lvalue_as_rvalue (gcc_jit_lvalue_access_field (bar, NULL, field));
+  gcc_jit_block_end_with_return (block, NULL, return_value);
+}
+
+/* { dg-final { jit-verify-output-file-was-created "" } } */
+/* { dg-final { jit-verify-assembler-output ".comm	foo,4,8" } } */
+/* { dg-final { jit-verify-assembler-output "movl	-16\\\(%rbp), %eax" } } */
-- 
2.26.2.7.g19db9cfb68.dirty

Re: [PATCH] libgccjit: Add support for setting the alignment [PR104293]

[David Malcolm]

On Sun, 2022-01-30 at 20:38 -0500, Antoni Boucher via Gcc-patches
wrote:
> Hi.
> This patch adds support for setting the alignment of variables in
> libgccjit.

Thanks.  Sorry about the delay in reviewing it.

> 
> I was wondering if I should change it so that it takes/returns bytes
> instead of bits.
> What do you think?

I'm not sure, but given that C refers to bytes for this:
  https://en.cppreference.com/w/c/language/object#Alignment
  https://en.cppreference.com/w/c/language/_Alignof
...I think bytes is the better choice, to maximize similarity with C.

Does anything support/need a fraction-of-a-byte alignment?  If so, then
bits would be the way to go.


> diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
> index 16cebe31a10..1957399dceb 100644
> --- a/gcc/jit/docs/topics/compatibility.rst
> +++ b/gcc/jit/docs/topics/compatibility.rst
> @@ -302,3 +302,13 @@ thread-local storage model of a variable:
>  section of a variable:
>  
>    * :func:`gcc_jit_lvalue_set_link_section`
> +
> +.. _LIBGCCJIT_ABI_24:
> +
> +``LIBGCCJIT_ABI_24``
> +-----------------------
> +``LIBGCCJIT_ABI_24`` covers the addition of functions to get and set the
> +alignement of a variable:
> +
> +  * :func:`gcc_jit_lvalue_set_alignment`
> +  * :func:`gcc_jit_lvalue_get_alignment`
> diff --git a/gcc/jit/docs/topics/expressions.rst b/gcc/jit/docs/topics/expressions.rst
> index 791a20398ca..0f5f5376d8c 100644
> --- a/gcc/jit/docs/topics/expressions.rst
> +++ b/gcc/jit/docs/topics/expressions.rst
> @@ -738,6 +738,45 @@ where the rvalue is computed by reading from the storage area.
>  
>        #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_link_section
>  
> +.. function:: void
> +              gcc_jit_lvalue_set_alignment (gcc_jit_lvalue *lvalue,
> +                                            int alignment)
> +
> +   Set the alignment of a variable.
> +   The parameter ``alignment`` is in bits. Analogous to:
> +
> +   .. code-block:: c
> +
> +     int variable __attribute__((aligned (16)));
> +
> +   in C, but in bits instead of bytes.

If we're doing it in bytes, this will need updating, of course.

Maybe rename the int param from "alignment" to "bytes" to make this
clearer.

Probably should be unsigned as well.

> +
> +   This entrypoint was added in :ref:`LIBGCCJIT_ABI_24`; you can test for
> +   its presence using
> +
> +   .. code-block:: c
> +
> +      #ifdef LIBGCCJIT_HAVE_ALIGNMENT
> +
> +.. function:: int
> +              gcc_jit_lvalue_get_alignment (gcc_jit_lvalue *lvalue)
> +
> +   Return the alignment of a variable set by ``gcc_jit_lvalue_set_alignment``, in bits.
> +   Return 0 if the alignment was not set. Analogous to:
> +
> +   .. code-block:: c
> +
> +     _Alignof (variable)
> +
> +   in C, but in bits instead of bytes.

Likewise this will need updating.

> +
> +   This entrypoint was added in :ref:`LIBGCCJIT_ABI_24`; you can test for
> +   its presence using
> +
> +   .. code-block:: c
> +
> +      #ifdef LIBGCCJIT_HAVE_ALIGNMENT
> +
>  Global variables
>  ****************
>

[...snip...]

> diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
> index 4c352e8c93d..e03f15ec9c8 100644
> --- a/gcc/jit/libgccjit.cc
> +++ b/gcc/jit/libgccjit.cc
> @@ -2649,6 +2649,31 @@ gcc_jit_lvalue_set_link_section (gcc_jit_lvalue *lvalue,
>    lvalue->set_link_section (section_name);
>  }
>  
> +/* Public entrypoint.  See description in libgccjit.h.
> +
> +   After error-checking, the real work is done by the
> +   gcc::jit::recording::lvalue::get_link_section method in jit-recording.cc.  */

Comment refers to wrong function.

> +
> +int
> +gcc_jit_lvalue_get_alignment (gcc_jit_lvalue *lvalue)
> +{
> +  RETURN_VAL_IF_FAIL (lvalue, 0, NULL, NULL, "NULL lvalue");
> +  return lvalue->get_alignment ();
> +}

Should this return unsigned?

> +
> +/* Public entrypoint.  See description in libgccjit.h.
> +
> +   After error-checking, the real work is done by the
> +   gcc::jit::recording::lvalue::set_alignment method in jit-recording.cc.  */
> +
> +void
> +gcc_jit_lvalue_set_alignment (gcc_jit_lvalue *lvalue,
> +			      int alignment)
> +{
> +  RETURN_IF_FAIL (lvalue, NULL, NULL, "NULL lvalue");

Should the alignment be unsigned?  What if the user passes in negative?

Does it have to be a power of two?  If so, ideally we should reject
non-power-of-two here.

> +  lvalue->set_alignment (alignment);
> +}
> +

[...snip...]

> diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
> index f373fd39ac7..df51e4fdd8c 100644
> --- a/gcc/jit/libgccjit.map
> +++ b/gcc/jit/libgccjit.map
> @@ -243,3 +243,21 @@ LIBGCCJIT_ABI_19 {
>      gcc_jit_context_new_union_constructor;
>      gcc_jit_global_set_initializer_rvalue;
>  } LIBGCCJIT_ABI_18;
> +
> +LIBGCCJIT_ABI_20 {
> +} LIBGCCJIT_ABI_19;
> +
> +LIBGCCJIT_ABI_21 {
> +} LIBGCCJIT_ABI_20;
> +
> +LIBGCCJIT_ABI_22 {
> +} LIBGCCJIT_ABI_21;
> +
> +LIBGCCJIT_ABI_23 {
> +} LIBGCCJIT_ABI_22;
> +
> +LIBGCCJIT_ABI_24 {
> +  global:
> +    gcc_jit_lvalue_set_alignment;
> +    gcc_jit_lvalue_get_alignment;
> +} LIBGCCJIT_ABI_23;

BTW, how much of a problem would it be to you if we changed the order
of some of these?

At this point the API numbering may be getting in the way of getting
some of the simpler changes into trunk.

> diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h
b/gcc/testsuite/jit.dg/all-non-failing-tests.h
> index 29afe064db6..72c46e81e51 100644
> --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
> +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
> @@ -306,6 +306,9 @@
>  #undef create_code
>  #undef verify_code
>  
> +/* test-setting-alignment.c: This can't be in the testcases array as it
> +   doesn't have a verify_code implementation.  */

My first though was that with an empty verify_code implementation it
might work, but I see that the test overrides the regular options to
avoid -O3, so it can't be part of the combined tests.

> +
>  /* test-string-literal.c */
>  #define create_code create_code_string_literal
>  #define verify_code verify_code_string_literal
> diff --git a/gcc/testsuite/jit.dg/test-setting-alignment.c b/gcc/testsuite/jit.dg/test-setting-alignment.c
> new file mode 100644
> index 00000000000..e87afbeacd3
> --- /dev/null
> +++ b/gcc/testsuite/jit.dg/test-setting-alignment.c
> @@ -0,0 +1,64 @@
> +#include <stdlib.h>
> +#include <stdio.h>
> +
> +#include "libgccjit.h"
> +
> +/* We don't want set_options() in harness.h to set -O3 so our little local
> +   is optimized away. */
> +#define TEST_ESCHEWS_SET_OPTIONS
> +static void set_options (gcc_jit_context *ctxt, const char *argv0)
> +{
> +}
> +
> +#define TEST_COMPILING_TO_FILE
> +#define OUTPUT_KIND      GCC_JIT_OUTPUT_KIND_ASSEMBLER
> +#define OUTPUT_FILENAME  "output-of-test-setting-alignment.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__((aligned (8)));
> +
> +     int main (void) {
> +        int bar __attribute__((aligned (16)));
> +     }
> +  */
> +  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_alignment(foo, 64);
> +
> +  gcc_jit_field *field = gcc_jit_context_new_field (ctxt,
> +    NULL,
> +    int_type,
> +    "a");
> +  gcc_jit_struct *struct_type =
> +    gcc_jit_context_new_struct_type(ctxt, NULL, "Type", 1, &field);
> +  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_lvalue *bar =
> +    gcc_jit_function_new_local (
> +      func_main, NULL,
> +      gcc_jit_struct_as_type (struct_type),
> +      "bar");
> +  gcc_jit_lvalue_set_alignment(bar, 128);
> +  gcc_jit_block *block = gcc_jit_function_new_block (func_main, NULL);
> +  /*gcc_jit_block_add_assignment (block, NULL, bar, zero);*/
> +  gcc_jit_rvalue *return_value =
> +      gcc_jit_lvalue_as_rvalue (gcc_jit_lvalue_access_field (bar, NULL, field));
> +  gcc_jit_block_end_with_return (block, NULL, return_value);
> +}
> +
> +/* { dg-final { jit-verify-output-file-was-created "" } } */
> +/* { dg-final { jit-verify-assembler-output ".comm	foo,4,8" } } */
> +/* { dg-final { jit-verify-assembler-output "movl	-16\\\(%rbp), %eax" } } */

The expected output from the test is x86 specific, so it needs something like:

  /* { dg-do compile { target x86_64-*-* } } */

at the top.

Also, there's no test coverage for gcc_jit_lvalue_get_alignment.


Hope the above is constructive; thanks again for the patch

Dave

Re: [PATCH] libgccjit: Add support for setting the alignment [PR104293]

[Antoni Boucher]

[-- Attachment #1: Type: text/plain, Size: 10612 bytes --]

Here's the updated patch.

On Fri, 2022-04-08 at 15:01 -0400, David Malcolm wrote:
> On Sun, 2022-01-30 at 20:38 -0500, Antoni Boucher via Gcc-patches
> wrote:
> > Hi.
> > This patch adds support for setting the alignment of variables in
> > libgccjit.
> 
> Thanks.  Sorry about the delay in reviewing it.
> 
> > 
> > I was wondering if I should change it so that it takes/returns
> > bytes
> > instead of bits.
> > What do you think?
> 
> I'm not sure, but given that C refers to bytes for this:
>   https://en.cppreference.com/w/c/language/object#Alignment
>   https://en.cppreference.com/w/c/language/_Alignof
> ...I think bytes is the better choice, to maximize similarity with C.

Ok, I updated it to use bytes.

> 
> Does anything support/need a fraction-of-a-byte alignment?  If so,
> then
> bits would be the way to go.
> 
> 
> > diff --git a/gcc/jit/docs/topics/compatibility.rst
> > b/gcc/jit/docs/topics/compatibility.rst
> > index 16cebe31a10..1957399dceb 100644
> > --- a/gcc/jit/docs/topics/compatibility.rst
> > +++ b/gcc/jit/docs/topics/compatibility.rst
> > @@ -302,3 +302,13 @@ thread-local storage model of a variable:
> >  section of a variable:
> >  
> >    * :func:`gcc_jit_lvalue_set_link_section`
> > +
> > +.. _LIBGCCJIT_ABI_24:
> > +
> > +``LIBGCCJIT_ABI_24``
> > +-----------------------
> > +``LIBGCCJIT_ABI_24`` covers the addition of functions to get and
> > set the
> > +alignement of a variable:
> > +
> > +  * :func:`gcc_jit_lvalue_set_alignment`
> > +  * :func:`gcc_jit_lvalue_get_alignment`
> > diff --git a/gcc/jit/docs/topics/expressions.rst
> > b/gcc/jit/docs/topics/expressions.rst
> > index 791a20398ca..0f5f5376d8c 100644
> > --- a/gcc/jit/docs/topics/expressions.rst
> > +++ b/gcc/jit/docs/topics/expressions.rst
> > @@ -738,6 +738,45 @@ where the rvalue is computed by reading from
> > the storage area.
> >  
> >        #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_link_section
> >  
> > +.. function:: void
> > +              gcc_jit_lvalue_set_alignment (gcc_jit_lvalue
> > *lvalue,
> > +                                            int alignment)
> > +
> > +   Set the alignment of a variable.
> > +   The parameter ``alignment`` is in bits. Analogous to:
> > +
> > +   .. code-block:: c
> > +
> > +     int variable __attribute__((aligned (16)));
> > +
> > +   in C, but in bits instead of bytes.
> 
> If we're doing it in bytes, this will need updating, of course.
> 
> Maybe rename the int param from "alignment" to "bytes" to make this
> clearer.
> 
> Probably should be unsigned as well.
> 
> > +
> > +   This entrypoint was added in :ref:`LIBGCCJIT_ABI_24`; you can
> > test for
> > +   its presence using
> > +
> > +   .. code-block:: c
> > +
> > +      #ifdef LIBGCCJIT_HAVE_ALIGNMENT
> > +
> > +.. function:: int
> > +              gcc_jit_lvalue_get_alignment (gcc_jit_lvalue
> > *lvalue)
> > +
> > +   Return the alignment of a variable set by
> > ``gcc_jit_lvalue_set_alignment``, in bits.
> > +   Return 0 if the alignment was not set. Analogous to:
> > +
> > +   .. code-block:: c
> > +
> > +     _Alignof (variable)
> > +
> > +   in C, but in bits instead of bytes.
> 
> Likewise this will need updating.
> 
> > +
> > +   This entrypoint was added in :ref:`LIBGCCJIT_ABI_24`; you can
> > test for
> > +   its presence using
> > +
> > +   .. code-block:: c
> > +
> > +      #ifdef LIBGCCJIT_HAVE_ALIGNMENT
> > +
> >  Global variables
> >  ****************
> > 
> 
> [...snip...]
> 
> > diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
> > index 4c352e8c93d..e03f15ec9c8 100644
> > --- a/gcc/jit/libgccjit.cc
> > +++ b/gcc/jit/libgccjit.cc
> > @@ -2649,6 +2649,31 @@ gcc_jit_lvalue_set_link_section
> > (gcc_jit_lvalue *lvalue,
> >    lvalue->set_link_section (section_name);
> >  }
> >  
> > +/* Public entrypoint.  See description in libgccjit.h.
> > +
> > +   After error-checking, the real work is done by the
> > +   gcc::jit::recording::lvalue::get_link_section method in jit-
> > recording.cc.  */
> 
> Comment refers to wrong function.
> 
> > +
> > +int
> > +gcc_jit_lvalue_get_alignment (gcc_jit_lvalue *lvalue)
> > +{
> > +  RETURN_VAL_IF_FAIL (lvalue, 0, NULL, NULL, "NULL lvalue");
> > +  return lvalue->get_alignment ();
> > +}
> 
> Should this return unsigned?
> 
> > +
> > +/* Public entrypoint.  See description in libgccjit.h.
> > +
> > +   After error-checking, the real work is done by the
> > +   gcc::jit::recording::lvalue::set_alignment method in jit-
> > recording.cc.  */
> > +
> > +void
> > +gcc_jit_lvalue_set_alignment (gcc_jit_lvalue *lvalue,
> > +                             int alignment)
> > +{
> > +  RETURN_IF_FAIL (lvalue, NULL, NULL, "NULL lvalue");
> 
> Should the alignment be unsigned?  What if the user passes in
> negative?
> 
> Does it have to be a power of two?  If so, ideally we should reject
> non-power-of-two here.
> 
> > +  lvalue->set_alignment (alignment);
> > +}
> > +
> 
> [...snip...]
> 
> > diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
> > index f373fd39ac7..df51e4fdd8c 100644
> > --- a/gcc/jit/libgccjit.map
> > +++ b/gcc/jit/libgccjit.map
> > @@ -243,3 +243,21 @@ LIBGCCJIT_ABI_19 {
> >      gcc_jit_context_new_union_constructor;
> >      gcc_jit_global_set_initializer_rvalue;
> >  } LIBGCCJIT_ABI_18;
> > +
> > +LIBGCCJIT_ABI_20 {
> > +} LIBGCCJIT_ABI_19;
> > +
> > +LIBGCCJIT_ABI_21 {
> > +} LIBGCCJIT_ABI_20;
> > +
> > +LIBGCCJIT_ABI_22 {
> > +} LIBGCCJIT_ABI_21;
> > +
> > +LIBGCCJIT_ABI_23 {
> > +} LIBGCCJIT_ABI_22;
> > +
> > +LIBGCCJIT_ABI_24 {
> > +  global:
> > +    gcc_jit_lvalue_set_alignment;
> > +    gcc_jit_lvalue_get_alignment;
> > +} LIBGCCJIT_ABI_23;
> 
> BTW, how much of a problem would it be to you if we changed the order
> of some of these?

That's not an issue: I have no problem changing the order.

> 
> At this point the API numbering may be getting in the way of getting
> some of the simpler changes into trunk.
> 
> > diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h
> b/gcc/testsuite/jit.dg/all-non-failing-tests.h
> > index 29afe064db6..72c46e81e51 100644
> > --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
> > +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
> > @@ -306,6 +306,9 @@
> >  #undef create_code
> >  #undef verify_code
> >  
> > +/* test-setting-alignment.c: This can't be in the testcases array
> > as it
> > +   doesn't have a verify_code implementation.  */
> 
> My first though was that with an empty verify_code implementation it
> might work, but I see that the test overrides the regular options to
> avoid -O3, so it can't be part of the combined tests.
> 
> > +
> >  /* test-string-literal.c */
> >  #define create_code create_code_string_literal
> >  #define verify_code verify_code_string_literal
> > diff --git a/gcc/testsuite/jit.dg/test-setting-alignment.c
> > b/gcc/testsuite/jit.dg/test-setting-alignment.c
> > new file mode 100644
> > index 00000000000..e87afbeacd3
> > --- /dev/null
> > +++ b/gcc/testsuite/jit.dg/test-setting-alignment.c
> > @@ -0,0 +1,64 @@
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +
> > +#include "libgccjit.h"
> > +
> > +/* We don't want set_options() in harness.h to set -O3 so our
> > little local
> > +   is optimized away. */
> > +#define TEST_ESCHEWS_SET_OPTIONS
> > +static void set_options (gcc_jit_context *ctxt, const char *argv0)
> > +{
> > +}
> > +
> > +#define TEST_COMPILING_TO_FILE
> > +#define OUTPUT_KIND      GCC_JIT_OUTPUT_KIND_ASSEMBLER
> > +#define OUTPUT_FILENAME  "output-of-test-setting-alignment.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__((aligned (8)));
> > +
> > +     int main (void) {
> > +        int bar __attribute__((aligned (16)));
> > +     }
> > +  */
> > +  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_alignment(foo, 64);
> > +
> > +  gcc_jit_field *field = gcc_jit_context_new_field (ctxt,
> > +    NULL,
> > +    int_type,
> > +    "a");
> > +  gcc_jit_struct *struct_type =
> > +    gcc_jit_context_new_struct_type(ctxt, NULL, "Type", 1,
> > &field);
> > +  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_lvalue *bar =
> > +    gcc_jit_function_new_local (
> > +      func_main, NULL,
> > +      gcc_jit_struct_as_type (struct_type),
> > +      "bar");
> > +  gcc_jit_lvalue_set_alignment(bar, 128);
> > +  gcc_jit_block *block = gcc_jit_function_new_block (func_main,
> > NULL);
> > +  /*gcc_jit_block_add_assignment (block, NULL, bar, zero);*/
> > +  gcc_jit_rvalue *return_value =
> > +      gcc_jit_lvalue_as_rvalue (gcc_jit_lvalue_access_field (bar,
> > NULL, field));
> > +  gcc_jit_block_end_with_return (block, NULL, return_value);
> > +}
> > +
> > +/* { dg-final { jit-verify-output-file-was-created "" } } */
> > +/* { dg-final { jit-verify-assembler-output ".comm     foo,4,8" }
> > } */
> > +/* { dg-final { jit-verify-assembler-output "movl      -
> > 16\\\(%rbp), %eax" } } */
> 
> The expected output from the test is x86 specific, so it needs
> something like:
> 
>   /* { dg-do compile { target x86_64-*-* } } */
> 
> at the top.
> 
> Also, there's no test coverage for gcc_jit_lvalue_get_alignment.
> 
> 
> Hope the above is constructive; thanks again for the patch
> 
> Dave
> 
> 


[-- Attachment #2: 0001-libgccjit-Add-support-for-setting-the-alignment-PR10.patch --]
[-- Type: text/x-patch, Size: 12018 bytes --]

From 75936fbf19ca66bb3e710e046babf0b3cbc6f738 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Tue, 25 Jan 2022 21:12:32 -0500
Subject: [PATCH] libgccjit: Add support for setting the alignment [PR104293]

2022-04-09  Antoni Boucher <bouanto@zoho.com>

gcc/jit/
	PR jit/104293
	* docs/topics/compatibility.rst (LIBGCCJIT_ABI_24): New ABI tag.
	* docs/topics/expressions.rst: Add documentation for the
	functions gcc_jit_lvalue_set_alignment and
	gcc_jit_lvalue_get_alignment.
	* jit-playback.h: New function (set_alignment).
	* jit-recording.cc: New function (set_alignment).
	* jit-recording.h: New functions (set_alignment, get_alignment)
	and new field (m_alignment).
	* libgccjit.cc: New functions (gcc_jit_lvalue_get_alignment,
	gcc_jit_lvalue_set_alignment)
	* libgccjit.h: New functions (gcc_jit_lvalue_get_alignment,
	gcc_jit_lvalue_set_alignment)
	* libgccjit.map (LIBGCCJIT_ABI_24): New ABI tag.

gcc/testsuite/
	PR jit/104293
	* jit.dg/all-non-failing-tests.h: Mention
	test-setting-alignment.
	* jit.dg/test-setting-alignment.c: New test.
---
 gcc/jit/docs/topics/compatibility.rst         | 10 +++
 gcc/jit/docs/topics/expressions.rst           | 39 +++++++++++
 gcc/jit/jit-playback.h                        |  7 ++
 gcc/jit/jit-recording.cc                      | 17 ++++-
 gcc/jit/jit-recording.h                       |  6 +-
 gcc/jit/libgccjit.cc                          | 26 ++++++++
 gcc/jit/libgccjit.h                           | 19 ++++++
 gcc/jit/libgccjit.map                         | 18 +++++
 gcc/testsuite/jit.dg/all-non-failing-tests.h  |  3 +
 gcc/testsuite/jit.dg/test-setting-alignment.c | 66 +++++++++++++++++++
 10 files changed, 208 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/jit.dg/test-setting-alignment.c

diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
index 16cebe31a10..1957399dceb 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -302,3 +302,13 @@ thread-local storage model of a variable:
 section of a variable:
 
   * :func:`gcc_jit_lvalue_set_link_section`
+
+.. _LIBGCCJIT_ABI_24:
+
+``LIBGCCJIT_ABI_24``
+-----------------------
+``LIBGCCJIT_ABI_24`` covers the addition of functions to get and set the
+alignement of a variable:
+
+  * :func:`gcc_jit_lvalue_set_alignment`
+  * :func:`gcc_jit_lvalue_get_alignment`
diff --git a/gcc/jit/docs/topics/expressions.rst b/gcc/jit/docs/topics/expressions.rst
index 791a20398ca..664b27dc81b 100644
--- a/gcc/jit/docs/topics/expressions.rst
+++ b/gcc/jit/docs/topics/expressions.rst
@@ -738,6 +738,45 @@ where the rvalue is computed by reading from the storage area.
 
       #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_link_section
 
+.. function:: void
+              gcc_jit_lvalue_set_alignment (gcc_jit_lvalue *lvalue,
+                                            unsigned bytes)
+
+   Set the alignment of a variable.
+   Analogous to:
+
+   .. code-block:: c
+
+     int variable __attribute__((aligned (16)));
+
+   in C.
+
+   This entrypoint was added in :ref:`LIBGCCJIT_ABI_24`; you can test for
+   its presence using
+
+   .. code-block:: c
+
+      #ifdef LIBGCCJIT_HAVE_ALIGNMENT
+
+.. function:: unsigned
+              gcc_jit_lvalue_get_alignment (gcc_jit_lvalue *lvalue)
+
+   Return the alignment of a variable set by ``gcc_jit_lvalue_set_alignment``.
+   Return 0 if the alignment was not set. Analogous to:
+
+   .. code-block:: c
+
+     _Alignof (variable)
+
+   in C.
+
+   This entrypoint was added in :ref:`LIBGCCJIT_ABI_24`; you can test for
+   its presence using
+
+   .. code-block:: c
+
+      #ifdef LIBGCCJIT_HAVE_ALIGNMENT
+
 Global variables
 ****************
 
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index c93d7055d43..d11053f66d9 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -701,6 +701,13 @@ public:
     set_decl_section_name (as_tree (), name);
   }
 
+  void
+  set_alignment (int alignment)
+  {
+      SET_DECL_ALIGN (as_tree (), alignment * BITS_PER_UNIT);
+      DECL_USER_ALIGN (as_tree ()) = 1;
+  }
+
 private:
   bool mark_addressable (location *loc);
 };
diff --git a/gcc/jit/jit-recording.cc b/gcc/jit/jit-recording.cc
index 1e3fadfacd7..bac43e57b4a 100644
--- a/gcc/jit/jit-recording.cc
+++ b/gcc/jit/jit-recording.cc
@@ -3807,6 +3807,11 @@ void recording::lvalue::set_link_section (const char *name)
   m_link_section = new_string (name);
 }
 
+void recording::lvalue::set_alignment (unsigned bytes)
+{
+  m_alignment = bytes;
+}
+
 /* The implementation of class gcc::jit::recording::param.  */
 
 /* Implementation of pure virtual hook recording::memento::replay_into
@@ -4673,6 +4678,9 @@ recording::global::replay_into (replayer *r)
   if (m_link_section != NULL)
     global->set_link_section (m_link_section->c_str ());
 
+  if (m_alignment != 0)
+    global->set_alignment (m_alignment);
+
   set_playback_obj (global);
 }
 
@@ -6343,11 +6351,16 @@ recording::function_pointer::write_reproducer (reproducer &r)
 void
 recording::local::replay_into (replayer *r)
 {
-  set_playback_obj (
+  playback::lvalue *local =
     m_func->playback_function ()
       ->new_local (playback_location (r, m_loc),
 		   m_type->playback_type (),
-		   playback_string (m_name)));
+		   playback_string (m_name));
+
+  if (m_alignment != 0)
+    local->set_alignment (m_alignment);
+
+  set_playback_obj (local);
 }
 
 /* Override the default implementation of
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 846d65cb202..3535f528887 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -1148,7 +1148,8 @@ public:
 	  type *type_)
     : rvalue (ctxt, loc, type_),
     m_tls_model (GCC_JIT_TLS_MODEL_NONE),
-    m_link_section (NULL)
+    m_link_section (NULL),
+    m_alignment (0)
     {}
 
   playback::lvalue *
@@ -1172,10 +1173,13 @@ public:
   virtual bool is_global () const { return false; }
   void set_tls_model (enum gcc_jit_tls_model model);
   void set_link_section (const char *name);
+  void set_alignment (unsigned bytes);
+  unsigned get_alignment () { return m_alignment; }
 
 protected:
   enum gcc_jit_tls_model m_tls_model;
   string *m_link_section;
+  unsigned m_alignment;
 };
 
 class param : public lvalue
diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
index 4c352e8c93d..0c592daeee9 100644
--- a/gcc/jit/libgccjit.cc
+++ b/gcc/jit/libgccjit.cc
@@ -2649,6 +2649,32 @@ gcc_jit_lvalue_set_link_section (gcc_jit_lvalue *lvalue,
   lvalue->set_link_section (section_name);
 }
 
+/* Public entrypoint.  See description in libgccjit.h.
+
+   After error-checking, the real work is done by the
+   gcc::jit::recording::lvalue::get_alignment method in jit-recording.cc.  */
+
+unsigned
+gcc_jit_lvalue_get_alignment (gcc_jit_lvalue *lvalue)
+{
+  RETURN_VAL_IF_FAIL (lvalue, 0, NULL, NULL, "NULL lvalue");
+  return lvalue->get_alignment ();
+}
+
+/* Public entrypoint.  See description in libgccjit.h.
+
+   After error-checking, the real work is done by the
+   gcc::jit::recording::lvalue::set_alignment method in jit-recording.cc.  */
+
+void
+gcc_jit_lvalue_set_alignment (gcc_jit_lvalue *lvalue,
+			      unsigned bytes)
+{
+  RETURN_IF_FAIL (lvalue, NULL, NULL, "NULL lvalue");
+  RETURN_IF_FAIL ((bytes & (bytes - 1)) == 0, NULL, NULL, "alignement is not a power of 2");
+  lvalue->set_alignment (bytes);
+}
+
 /* 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 2a5ffacb1fe..263b98728b0 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -1277,6 +1277,25 @@ extern void
 gcc_jit_lvalue_set_link_section (gcc_jit_lvalue *lvalue,
 			    const char *section_name);
 
+#define LIBGCCJIT_HAVE_ALIGNMENT
+
+/* Set the alignment of a variable.
+
+   This API entrypoint was added in LIBGCCJIT_ABI_24; you can test for its
+   presence using
+     #ifdef LIBGCCJIT_HAVE_ALIGNMENT  */
+extern void
+gcc_jit_lvalue_set_alignment (gcc_jit_lvalue *lvalue,
+			      unsigned bytes);
+
+/* Get the alignment of a variable.
+
+   This API entrypoint was added in LIBGCCJIT_ABI_24; you can test for its
+   presence using
+     #ifdef LIBGCCJIT_HAVE_ALIGNMENT  */
+extern unsigned
+gcc_jit_lvalue_get_alignment (gcc_jit_lvalue *lvalue);
+
 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 f373fd39ac7..df51e4fdd8c 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -243,3 +243,21 @@ LIBGCCJIT_ABI_19 {
     gcc_jit_context_new_union_constructor;
     gcc_jit_global_set_initializer_rvalue;
 } LIBGCCJIT_ABI_18;
+
+LIBGCCJIT_ABI_20 {
+} LIBGCCJIT_ABI_19;
+
+LIBGCCJIT_ABI_21 {
+} LIBGCCJIT_ABI_20;
+
+LIBGCCJIT_ABI_22 {
+} LIBGCCJIT_ABI_21;
+
+LIBGCCJIT_ABI_23 {
+} LIBGCCJIT_ABI_22;
+
+LIBGCCJIT_ABI_24 {
+  global:
+    gcc_jit_lvalue_set_alignment;
+    gcc_jit_lvalue_get_alignment;
+} LIBGCCJIT_ABI_23;
diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index 29afe064db6..72c46e81e51 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -306,6 +306,9 @@
 #undef create_code
 #undef verify_code
 
+/* test-setting-alignment.c: This can't be in the testcases array as it
+   doesn't have a verify_code implementation.  */
+
 /* test-string-literal.c */
 #define create_code create_code_string_literal
 #define verify_code verify_code_string_literal
diff --git a/gcc/testsuite/jit.dg/test-setting-alignment.c b/gcc/testsuite/jit.dg/test-setting-alignment.c
new file mode 100644
index 00000000000..8489df9c6b9
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-setting-alignment.c
@@ -0,0 +1,66 @@
+/* { dg-do compile { target x86_64-*-* } } */
+
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "libgccjit.h"
+
+/* We don't want set_options() in harness.h to set -O3 so our little local
+   is optimized away. */
+#define TEST_ESCHEWS_SET_OPTIONS
+static void set_options (gcc_jit_context *ctxt, const char *argv0)
+{
+}
+
+#define TEST_COMPILING_TO_FILE
+#define OUTPUT_KIND      GCC_JIT_OUTPUT_KIND_ASSEMBLER
+#define OUTPUT_FILENAME  "output-of-test-setting-alignment.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__((aligned (8)));
+
+     int main (void) {
+        int bar __attribute__((aligned (16)));
+     }
+  */
+  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_alignment(foo, 8);
+
+  gcc_jit_field *field = gcc_jit_context_new_field (ctxt,
+    NULL,
+    int_type,
+    "a");
+  gcc_jit_struct *struct_type =
+    gcc_jit_context_new_struct_type(ctxt, NULL, "Type", 1, &field);
+  gcc_jit_function *func_main =
+    gcc_jit_context_new_function (ctxt, NULL,
+				  GCC_JIT_FUNCTION_EXPORTED,
+				  int_type,
+				  "main",
+				  0, NULL,
+				  0);
+  gcc_jit_lvalue *bar =
+    gcc_jit_function_new_local (
+      func_main, NULL,
+      gcc_jit_struct_as_type (struct_type),
+      "bar");
+  CHECK_VALUE (gcc_jit_lvalue_get_alignment (bar), 0);
+  gcc_jit_lvalue_set_alignment (bar, 16);
+  CHECK_VALUE (gcc_jit_lvalue_get_alignment (bar), 16);
+  gcc_jit_block *block = gcc_jit_function_new_block (func_main, NULL);
+  gcc_jit_rvalue *return_value =
+      gcc_jit_lvalue_as_rvalue (gcc_jit_lvalue_access_field (bar, NULL, field));
+  gcc_jit_block_end_with_return (block, NULL, return_value);
+}
+
+/* { dg-final { jit-verify-output-file-was-created "" } } */
+/* { dg-final { jit-verify-assembler-output ".comm	foo,4,8" } } */
+/* { dg-final { jit-verify-assembler-output "movl	-16\\\(%rbp\\\), %eax" } } */
-- 
2.26.2.7.g19db9cfb68.dirty

Re: [PATCH] libgccjit: Add support for setting the alignment [PR104293]

[David Malcolm]

On Sat, 2022-04-09 at 13:50 -0400, Antoni Boucher wrote:
> Here's the updated patch.
> 
> On Fri, 2022-04-08 at 15:01 -0400, David Malcolm wrote:
> > On Sun, 2022-01-30 at 20:38 -0500, Antoni Boucher via Gcc-patches
> > wrote:
> > > Hi.
> > > This patch adds support for setting the alignment of variables in
> > > libgccjit.
> > 
> > Thanks.  Sorry about the delay in reviewing it.
> > 
> > > 
> > > I was wondering if I should change it so that it takes/returns
> > > bytes
> > > instead of bits.
> > > What do you think?
> > 
> > I'm not sure, but given that C refers to bytes for this:
> >   https://en.cppreference.com/w/c/language/object#Alignment
> >   https://en.cppreference.com/w/c/language/_Alignof
> > ...I think bytes is the better choice, to maximize similarity with C.
> 
> Ok, I updated it to use bytes.

Thanks.

I updated the patch slightly:
* fixed up some hunks that didn't quite apply
* tweaked the comment in all-non-failing-tests.h
* some whitespace fixes, and a couple of "alignement" to "alignment"
spelling fixes
* added an ", in bytes" to the doc for gcc_jit_lvalue_set_alignment.
* regenerated .texinfo from .rst

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

I've pushed the patch to trunk for GCC 12 as r12-8120-g6e5ad1cc24a315.

https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=6e5ad1cc24a315d07f24c95d76c269cafe2a8182

Thanks again for all the patches
Dave



> > 
> > Does anything support/need a fraction-of-a-byte alignment?  If so,
> > then
> > bits would be the way to go.
> > 
> > 
> > > diff --git a/gcc/jit/docs/topics/compatibility.rst
> > > b/gcc/jit/docs/topics/compatibility.rst
> > > index 16cebe31a10..1957399dceb 100644
> > > --- a/gcc/jit/docs/topics/compatibility.rst
> > > +++ b/gcc/jit/docs/topics/compatibility.rst
> > > @@ -302,3 +302,13 @@ thread-local storage model of a variable:
> > >  section of a variable:
> > >  
> > >    * :func:`gcc_jit_lvalue_set_link_section`
> > > +
> > > +.. _LIBGCCJIT_ABI_24:
> > > +
> > > +``LIBGCCJIT_ABI_24``
> > > +-----------------------
> > > +``LIBGCCJIT_ABI_24`` covers the addition of functions to get and
> > > set the
> > > +alignement of a variable:
> > > +
> > > +  * :func:`gcc_jit_lvalue_set_alignment`
> > > +  * :func:`gcc_jit_lvalue_get_alignment`
> > > diff --git a/gcc/jit/docs/topics/expressions.rst
> > > b/gcc/jit/docs/topics/expressions.rst
> > > index 791a20398ca..0f5f5376d8c 100644
> > > --- a/gcc/jit/docs/topics/expressions.rst
> > > +++ b/gcc/jit/docs/topics/expressions.rst
> > > @@ -738,6 +738,45 @@ where the rvalue is computed by reading from
> > > the storage area.
> > >  
> > >        #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_link_section
> > >  
> > > +.. function:: void
> > > +              gcc_jit_lvalue_set_alignment (gcc_jit_lvalue
> > > *lvalue,
> > > +                                            int alignment)
> > > +
> > > +   Set the alignment of a variable.
> > > +   The parameter ``alignment`` is in bits. Analogous to:
> > > +
> > > +   .. code-block:: c
> > > +
> > > +     int variable __attribute__((aligned (16)));
> > > +
> > > +   in C, but in bits instead of bytes.
> > 
> > If we're doing it in bytes, this will need updating, of course.
> > 
> > Maybe rename the int param from "alignment" to "bytes" to make this
> > clearer.
> > 
> > Probably should be unsigned as well.
> > 
> > > +
> > > +   This entrypoint was added in :ref:`LIBGCCJIT_ABI_24`; you can
> > > test for
> > > +   its presence using
> > > +
> > > +   .. code-block:: c
> > > +
> > > +      #ifdef LIBGCCJIT_HAVE_ALIGNMENT
> > > +
> > > +.. function:: int
> > > +              gcc_jit_lvalue_get_alignment (gcc_jit_lvalue
> > > *lvalue)
> > > +
> > > +   Return the alignment of a variable set by
> > > ``gcc_jit_lvalue_set_alignment``, in bits.
> > > +   Return 0 if the alignment was not set. Analogous to:
> > > +
> > > +   .. code-block:: c
> > > +
> > > +     _Alignof (variable)
> > > +
> > > +   in C, but in bits instead of bytes.
> > 
> > Likewise this will need updating.
> > 
> > > +
> > > +   This entrypoint was added in :ref:`LIBGCCJIT_ABI_24`; you can
> > > test for
> > > +   its presence using
> > > +
> > > +   .. code-block:: c
> > > +
> > > +      #ifdef LIBGCCJIT_HAVE_ALIGNMENT
> > > +
> > >  Global variables
> > >  ****************
> > > 
> > 
> > [...snip...]
> > 
> > > diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
> > > index 4c352e8c93d..e03f15ec9c8 100644
> > > --- a/gcc/jit/libgccjit.cc
> > > +++ b/gcc/jit/libgccjit.cc
> > > @@ -2649,6 +2649,31 @@ gcc_jit_lvalue_set_link_section
> > > (gcc_jit_lvalue *lvalue,
> > >    lvalue->set_link_section (section_name);
> > >  }
> > >  
> > > +/* Public entrypoint.  See description in libgccjit.h.
> > > +
> > > +   After error-checking, the real work is done by the
> > > +   gcc::jit::recording::lvalue::get_link_section method in jit-
> > > recording.cc.  */
> > 
> > Comment refers to wrong function.
> > 
> > > +
> > > +int
> > > +gcc_jit_lvalue_get_alignment (gcc_jit_lvalue *lvalue)
> > > +{
> > > +  RETURN_VAL_IF_FAIL (lvalue, 0, NULL, NULL, "NULL lvalue");
> > > +  return lvalue->get_alignment ();
> > > +}
> > 
> > Should this return unsigned?
> > 
> > > +
> > > +/* Public entrypoint.  See description in libgccjit.h.
> > > +
> > > +   After error-checking, the real work is done by the
> > > +   gcc::jit::recording::lvalue::set_alignment method in jit-
> > > recording.cc.  */
> > > +
> > > +void
> > > +gcc_jit_lvalue_set_alignment (gcc_jit_lvalue *lvalue,
> > > +                             int alignment)
> > > +{
> > > +  RETURN_IF_FAIL (lvalue, NULL, NULL, "NULL lvalue");
> > 
> > Should the alignment be unsigned?  What if the user passes in
> > negative?
> > 
> > Does it have to be a power of two?  If so, ideally we should reject
> > non-power-of-two here.
> > 
> > > +  lvalue->set_alignment (alignment);
> > > +}
> > > +
> > 
> > [...snip...]
> > 
> > > diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
> > > index f373fd39ac7..df51e4fdd8c 100644
> > > --- a/gcc/jit/libgccjit.map
> > > +++ b/gcc/jit/libgccjit.map
> > > @@ -243,3 +243,21 @@ LIBGCCJIT_ABI_19 {
> > >      gcc_jit_context_new_union_constructor;
> > >      gcc_jit_global_set_initializer_rvalue;
> > >  } LIBGCCJIT_ABI_18;
> > > +
> > > +LIBGCCJIT_ABI_20 {
> > > +} LIBGCCJIT_ABI_19;
> > > +
> > > +LIBGCCJIT_ABI_21 {
> > > +} LIBGCCJIT_ABI_20;
> > > +
> > > +LIBGCCJIT_ABI_22 {
> > > +} LIBGCCJIT_ABI_21;
> > > +
> > > +LIBGCCJIT_ABI_23 {
> > > +} LIBGCCJIT_ABI_22;
> > > +
> > > +LIBGCCJIT_ABI_24 {
> > > +  global:
> > > +    gcc_jit_lvalue_set_alignment;
> > > +    gcc_jit_lvalue_get_alignment;
> > > +} LIBGCCJIT_ABI_23;
> > 
> > BTW, how much of a problem would it be to you if we changed the order
> > of some of these?
> 
> That's not an issue: I have no problem changing the order.
> 
> > 
> > At this point the API numbering may be getting in the way of getting
> > some of the simpler changes into trunk.
> > 
> > > diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h
> > b/gcc/testsuite/jit.dg/all-non-failing-tests.h
> > > index 29afe064db6..72c46e81e51 100644
> > > --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
> > > +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
> > > @@ -306,6 +306,9 @@
> > >  #undef create_code
> > >  #undef verify_code
> > >  
> > > +/* test-setting-alignment.c: This can't be in the testcases array
> > > as it
> > > +   doesn't have a verify_code implementation.  */
> > 
> > My first though was that with an empty verify_code implementation it
> > might work, but I see that the test overrides the regular options to
> > avoid -O3, so it can't be part of the combined tests.
> > 
> > > +
> > >  /* test-string-literal.c */
> > >  #define create_code create_code_string_literal
> > >  #define verify_code verify_code_string_literal
> > > diff --git a/gcc/testsuite/jit.dg/test-setting-alignment.c
> > > b/gcc/testsuite/jit.dg/test-setting-alignment.c
> > > new file mode 100644
> > > index 00000000000..e87afbeacd3
> > > --- /dev/null
> > > +++ b/gcc/testsuite/jit.dg/test-setting-alignment.c
> > > @@ -0,0 +1,64 @@
> > > +#include <stdlib.h>
> > > +#include <stdio.h>
> > > +
> > > +#include "libgccjit.h"
> > > +
> > > +/* We don't want set_options() in harness.h to set -O3 so our
> > > little local
> > > +   is optimized away. */
> > > +#define TEST_ESCHEWS_SET_OPTIONS
> > > +static void set_options (gcc_jit_context *ctxt, const char *argv0)
> > > +{
> > > +}
> > > +
> > > +#define TEST_COMPILING_TO_FILE
> > > +#define OUTPUT_KIND      GCC_JIT_OUTPUT_KIND_ASSEMBLER
> > > +#define OUTPUT_FILENAME  "output-of-test-setting-alignment.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__((aligned (8)));
> > > +
> > > +     int main (void) {
> > > +        int bar __attribute__((aligned (16)));
> > > +     }
> > > +  */
> > > +  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_alignment(foo, 64);
> > > +
> > > +  gcc_jit_field *field = gcc_jit_context_new_field (ctxt,
> > > +    NULL,
> > > +    int_type,
> > > +    "a");
> > > +  gcc_jit_struct *struct_type =
> > > +    gcc_jit_context_new_struct_type(ctxt, NULL, "Type", 1,
> > > &field);
> > > +  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_lvalue *bar =
> > > +    gcc_jit_function_new_local (
> > > +      func_main, NULL,
> > > +      gcc_jit_struct_as_type (struct_type),
> > > +      "bar");
> > > +  gcc_jit_lvalue_set_alignment(bar, 128);
> > > +  gcc_jit_block *block = gcc_jit_function_new_block (func_main,
> > > NULL);
> > > +  /*gcc_jit_block_add_assignment (block, NULL, bar, zero);*/
> > > +  gcc_jit_rvalue *return_value =
> > > +      gcc_jit_lvalue_as_rvalue (gcc_jit_lvalue_access_field (bar,
> > > NULL, field));
> > > +  gcc_jit_block_end_with_return (block, NULL, return_value);
> > > +}
> > > +
> > > +/* { dg-final { jit-verify-output-file-was-created "" } } */
> > > +/* { dg-final { jit-verify-assembler-output ".comm     foo,4,8" }
> > > } */
> > > +/* { dg-final { jit-verify-assembler-output "movl      -
> > > 16\\\(%rbp), %eax" } } */
> > 
> > The expected output from the test is x86 specific, so it needs
> > something like:
> > 
> >   /* { dg-do compile { target x86_64-*-* } } */
> > 
> > at the top.
> > 
> > Also, there's no test coverage for gcc_jit_lvalue_get_alignment.
> > 
> > 
> > Hope the above is constructive; thanks again for the patch
> > 
> > Dave
> > 
> > 
> 

Re: [PATCH] libgccjit: Add support for setting the alignment [PR104293]

[Bernhard Reutner-Fischer]

On 12 April 2022 23:51:34 CEST, David Malcolm via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>On Sat, 2022-04-09 at 13:50 -0400, Antoni Boucher wrote:
>> Here's the updated patch.
>> 

>I've pushed the patch to trunk for GCC 12 as r12-8120-g6e5ad1cc24a315.
>
>https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=6e5ad1cc24a315d07f24c95d76c269cafe2a8182


>> > > +   in C, but in bits instead of bytes.
>> > 
>> > If we're doing it in bytes, this will need updating, of course.
>> > 
>> > Maybe rename the int param from "alignment" to "bytes" to make this
>> > clearer.
>> > 
>> > Probably should be unsigned as well.

>> > > +void
>> > > +gcc_jit_lvalue_set_alignment (gcc_jit_lvalue *lvalue,
>> > > +                             int alignment)
>> > > +{
>> > > +  RETURN_IF_FAIL (lvalue, NULL, NULL, "NULL lvalue");
>> > 
>> > Should the alignment be unsigned?  What if the user passes in
>> > negative?
>> > 
>> > Does it have to be a power of two?  If so, ideally we should reject
>> > non-power-of-two here.

The wrapper was changed to unsigned bytes.
set_alignment still takes int bytes AFAICS?

Now I think there are or may be machine dependent max alignment, aren't there? If there are, they are enforced later in the pipeline I assume?

thanks,

Re: [PATCH] libgccjit: Add support for setting the alignment [PR104293]

[Marc Nieper-Wißkirchen]

I haven't checked the cause of the bug reported here [1]. It may
either be in the struct constructor code or in the new code for
alignments, so this email goes to both Antoni and Petter. Speaking of
struct constructors, I have found another bug [2]

Thanks,

Marc

--

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105296
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105286

Am Mi., 13. Apr. 2022 um 13:47 Uhr schrieb Bernhard Reutner-Fischer
via Jit <jit@gcc.gnu.org>:
>
> On 12 April 2022 23:51:34 CEST, David Malcolm via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> >On Sat, 2022-04-09 at 13:50 -0400, Antoni Boucher wrote:
> >> Here's the updated patch.
> >>
>
> >I've pushed the patch to trunk for GCC 12 as r12-8120-g6e5ad1cc24a315.
> >
> >https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=6e5ad1cc24a315d07f24c95d76c269cafe2a8182
>
>
> >> > > +   in C, but in bits instead of bytes.
> >> >
> >> > If we're doing it in bytes, this will need updating, of course.
> >> >
> >> > Maybe rename the int param from "alignment" to "bytes" to make this
> >> > clearer.
> >> >
> >> > Probably should be unsigned as well.
>
> >> > > +void
> >> > > +gcc_jit_lvalue_set_alignment (gcc_jit_lvalue *lvalue,
> >> > > +                             int alignment)
> >> > > +{
> >> > > +  RETURN_IF_FAIL (lvalue, NULL, NULL, "NULL lvalue");
> >> >
> >> > Should the alignment be unsigned?  What if the user passes in
> >> > negative?
> >> >
> >> > Does it have to be a power of two?  If so, ideally we should reject
> >> > non-power-of-two here.
>
> The wrapper was changed to unsigned bytes.
> set_alignment still takes int bytes AFAICS?
>
> Now I think there are or may be machine dependent max alignment, aren't there? If there are, they are enforced later in the pipeline I assume?
>
> thanks,

Re: [PATCH] libgccjit: Add support for setting the alignment [PR104293]

[Antoni Boucher]

[-- Attachment #1: Type: text/plain, Size: 2472 bytes --]

It seems there are some errors in the struct constructor code.
The attached patch seems to fix the issue.

The code from this bug report doesn't use the new API for alignment
(gcc_jit_lvalue_set_alignment); it uses the former one
(gcc_jit_type_get_aligned).
Perhaps it would be worth testing with the new API.

On Sat, 2022-04-16 at 22:26 +0200, Marc Nieper-Wißkirchen wrote:
> I haven't checked the cause of the bug reported here [1]. It may
> either be in the struct constructor code or in the new code for
> alignments, so this email goes to both Antoni and Petter. Speaking of
> struct constructors, I have found another bug [2]
> 
> Thanks,
> 
> Marc
> 
> --
> 
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105296
> [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105286
> 
> Am Mi., 13. Apr. 2022 um 13:47 Uhr schrieb Bernhard Reutner-Fischer
> via Jit <jit@gcc.gnu.org>:
> > 
> > On 12 April 2022 23:51:34 CEST, David Malcolm via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> > > On Sat, 2022-04-09 at 13:50 -0400, Antoni Boucher wrote:
> > > > Here's the updated patch.
> > > > 
> > 
> > > I've pushed the patch to trunk for GCC 12 as r12-8120-
> > > g6e5ad1cc24a315.
> > > 
> > > https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=6e5ad1cc24a315d07f24c95d76c269cafe2a8182
> > 
> > 
> > > > > > +   in C, but in bits instead of bytes.
> > > > > 
> > > > > If we're doing it in bytes, this will need updating, of
> > > > > course.
> > > > > 
> > > > > Maybe rename the int param from "alignment" to "bytes" to
> > > > > make this
> > > > > clearer.
> > > > > 
> > > > > Probably should be unsigned as well.
> > 
> > > > > > +void
> > > > > > +gcc_jit_lvalue_set_alignment (gcc_jit_lvalue *lvalue,
> > > > > > +                             int alignment)
> > > > > > +{
> > > > > > +  RETURN_IF_FAIL (lvalue, NULL, NULL, "NULL lvalue");
> > > > > 
> > > > > Should the alignment be unsigned?  What if the user passes in
> > > > > negative?
> > > > > 
> > > > > Does it have to be a power of two?  If so, ideally we should
> > > > > reject
> > > > > non-power-of-two here.
> > 
> > The wrapper was changed to unsigned bytes.
> > set_alignment still takes int bytes AFAICS?
> > 
> > Now I think there are or may be machine dependent max alignment,
> > aren't there? If there are, they are enforced later in the pipeline
> > I assume?
> > 
> > thanks,


[-- Attachment #2: 0001-Fix-usage-of-aligned-type-in-struct-constructor.patch --]
[-- Type: text/x-patch, Size: 1769 bytes --]

From 0bbab5376cb88564b33a6bbdeaf3187f802c24c8 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Sat, 16 Apr 2022 22:25:41 -0400
Subject: [PATCH] Fix usage of aligned type in struct constructor

---
 gcc/jit/libgccjit.cc | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
index ce54cc82a7a..a417e40d728 100644
--- a/gcc/jit/libgccjit.cc
+++ b/gcc/jit/libgccjit.cc
@@ -1414,16 +1414,16 @@ gcc_jit_context_new_struct_constructor (gcc_jit_context *ctxt,
   JIT_LOG_FUNC (ctxt->get_logger ());
   RETURN_NULL_IF_FAIL (type, ctxt, loc, "NULL type");
 
-  RETURN_NULL_IF_FAIL_PRINTF1 (type->is_struct (),
+  struct_* struct_type = type->is_struct ();
+  RETURN_NULL_IF_FAIL_PRINTF1 (struct_type,
 			       ctxt, loc,
 			       "constructor type is not a struct: %s",
 			       type->get_debug_string ());
 
-  compound_type *ct = reinterpret_cast<compound_type *>(type);
-  gcc::jit::recording::fields *fields_struct = ct->get_fields ();
+  gcc::jit::recording::fields *fields_struct = struct_type->get_fields ();
   size_t n_fields = fields_struct->length ();
 
-  RETURN_NULL_IF_FAIL_PRINTF1 (ct->has_known_size (),
+  RETURN_NULL_IF_FAIL_PRINTF1 (struct_type->has_known_size (),
 			       ctxt, loc,
 			       "struct can't be opaque: %s",
 			       type->get_debug_string ());
@@ -1472,7 +1472,7 @@ gcc_jit_context_new_struct_constructor (gcc_jit_context *ctxt,
 
 	  RETURN_NULL_IF_FAIL_PRINTF3 (
 	    f->get_container () ==
-	    static_cast<gcc::jit::recording::type*>(type),
+	    static_cast<gcc::jit::recording::type*>(struct_type),
 	    ctxt, loc,
 	    "field object at index %zu (%s), was not used when creating "
 	    "the %s",
-- 
2.26.2.7.g19db9cfb68.dirty