[PATCH] libgccjit: Support signed char flag

[PATCH] libgccjit: Support signed char flag

[Antoni Boucher]

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

Hi.
This patch adds support for the -fsigned-char flag.
I'm not sure how to test it since I stumbled upon this bug when I found
this other bug (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107863)
which is now fixed.
Any idea how I could test this patch?
Thanks for the review.

[-- Attachment #2: 0001-libgccjit-Support-signed-char-flag.patch --]
[-- Type: text/x-patch, Size: 875 bytes --]

From 45719be81ab71983ab10ecb67139eaf02955e4db Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Mon, 3 Oct 2022 19:11:39 -0400
Subject: [PATCH] libgccjit: Support signed char flag

gcc/jit/ChangeLog:

	* dummy-frontend.cc (jit_langhook_init): Send flag_signed_char
	argument to build_common_tree_nodes.
---
 gcc/jit/dummy-frontend.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/jit/dummy-frontend.cc b/gcc/jit/dummy-frontend.cc
index 9f71bca44b1..11615b30f40 100644
--- a/gcc/jit/dummy-frontend.cc
+++ b/gcc/jit/dummy-frontend.cc
@@ -607,7 +607,7 @@ jit_langhook_init (void)
   diagnostic_starter (global_dc) = jit_begin_diagnostic;
   diagnostic_finalizer (global_dc) = jit_end_diagnostic;
 
-  build_common_tree_nodes (false);
+  build_common_tree_nodes (flag_signed_char);
 
   build_common_builtin_nodes ();
 
-- 
2.43.0

Re: [PATCH] libgccjit: Support signed char flag

[David Malcolm]

On Thu, 2023-12-21 at 08:42 -0500, Antoni Boucher wrote:
> Hi.
> This patch adds support for the -fsigned-char flag.

Thanks.  The patch looks correct to me.

> I'm not sure how to test it since I stumbled upon this bug when I
> found
> this other bug (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107863)
> which is now fixed.
> Any idea how I could test this patch?

We already document that GCC_JIT_TYPE_CHAR has "some signedness".  The
bug being fixed here is that gcc_jit_context compilations were always
treating "char" as unsigned, regardless of the value of -fsigned-char
(either from the target's default, or as a context option), when it
makes more sense to follow the C frontend's behavior.

So perhaps jit-written code with a context that has -fsigned-char as an
option (via gcc_jit_context_add_command_line_option), and which
promotes a negative char to a signed int, and then returns the result
as an int?  Presumably if we're erroneously forcing "char" to be
unsigned, the int will be in the range 0x80 to 0xff, rather that being
negative.

Dave

Re: [PATCH] libgccjit: Support signed char flag

[Antoni Boucher]

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

Thanks for the review and idea.

Here's the updated patch. I added a test, but I could not set -fsigned-
char as this is not an option accepted by the jit frontend.
However, the test still works in the sense that it fails without this
patch and passes with it.
I'm just wondering if it would pass on all targets or if I should add a
target filtering directive to only execute on some target.
What do you think?

On Tue, 2024-01-09 at 11:01 -0500, David Malcolm wrote:
> On Thu, 2023-12-21 at 08:42 -0500, Antoni Boucher wrote:
> > Hi.
> > This patch adds support for the -fsigned-char flag.
> 
> Thanks.  The patch looks correct to me.
> 
> > I'm not sure how to test it since I stumbled upon this bug when I
> > found
> > this other bug
> > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107863)
> > which is now fixed.
> > Any idea how I could test this patch?
> 
> We already document that GCC_JIT_TYPE_CHAR has "some signedness". 
> The
> bug being fixed here is that gcc_jit_context compilations were always
> treating "char" as unsigned, regardless of the value of -fsigned-char
> (either from the target's default, or as a context option), when it
> makes more sense to follow the C frontend's behavior.
> 
> So perhaps jit-written code with a context that has -fsigned-char as
> an
> option (via gcc_jit_context_add_command_line_option), and which
> promotes a negative char to a signed int, and then returns the result
> as an int?  Presumably if we're erroneously forcing "char" to be
> unsigned, the int will be in the range 0x80 to 0xff, rather that
> being
> negative.
> 
> Dave
> 


[-- Attachment #2: 0001-libgccjit-Support-signed-char-flag.patch --]
[-- Type: text/x-patch, Size: 3711 bytes --]

From 57d4bd695bcb16c54ecbe05346282f5dc270c30a Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Mon, 3 Oct 2022 19:11:39 -0400
Subject: [PATCH] libgccjit: Support signed char flag

gcc/jit/ChangeLog:

	* dummy-frontend.cc (jit_langhook_init): Send flag_signed_char
	argument to build_common_tree_nodes.

gcc/testsuite/ChangeLog:

	* jit.dg/all-non-failing-tests.h: Add test-signed-char.c.
	* jit.dg/test-signed-char.c: New test.
---
 gcc/jit/dummy-frontend.cc                    |  2 +-
 gcc/testsuite/jit.dg/all-non-failing-tests.h | 10 ++++
 gcc/testsuite/jit.dg/test-signed-char.c      | 52 ++++++++++++++++++++
 3 files changed, 63 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/jit.dg/test-signed-char.c

diff --git a/gcc/jit/dummy-frontend.cc b/gcc/jit/dummy-frontend.cc
index dbeeacd17a8..dc1347b714a 100644
--- a/gcc/jit/dummy-frontend.cc
+++ b/gcc/jit/dummy-frontend.cc
@@ -1029,7 +1029,7 @@ jit_langhook_init (void)
   diagnostic_starter (global_dc) = jit_begin_diagnostic;
   diagnostic_finalizer (global_dc) = jit_end_diagnostic;
 
-  build_common_tree_nodes (false);
+  build_common_tree_nodes (flag_signed_char);
 
   build_common_builtin_nodes ();
 
diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index 14a0a321550..404377a4df0 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -353,6 +353,13 @@
 /* test-setting-alignment.c: This can't be in the testcases array as it
    is target-specific.  */
 
+/* test-signed-char.c */
+#define create_code create_code_signed_char
+#define verify_code verify_code_signed_char
+#include "test-signed-char.c"
+#undef create_code
+#undef verify_code
+
 /* test-sizeof.c */
 #define create_code create_code_sizeof
 #define verify_code verify_code_sizeof
@@ -560,6 +567,9 @@ const struct testcase testcases[] = {
   {"reflection",
    create_code_reflection ,
    verify_code_reflection },
+  {"signed-char",
+   create_code_signed_char,
+   verify_code_signed_char},
   {"sizeof",
    create_code_sizeof,
    verify_code_sizeof},
diff --git a/gcc/testsuite/jit.dg/test-signed-char.c b/gcc/testsuite/jit.dg/test-signed-char.c
new file mode 100644
index 00000000000..c12b41d92cc
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-signed-char.c
@@ -0,0 +1,52 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <stddef.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 test_signed_char ()
+        {
+            char val = -2;
+            return (int) val;
+        }
+    */
+  gcc_jit_type *char_type =
+    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_CHAR);
+  gcc_jit_type *int_type =
+    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
+
+  gcc_jit_function *test_fn =
+    gcc_jit_context_new_function (ctxt, NULL,
+				  GCC_JIT_FUNCTION_EXPORTED,
+				  int_type,
+				  "test_signed_char",
+				  0, NULL,
+				  0);
+
+  gcc_jit_block *block = gcc_jit_function_new_block(test_fn, "entry");
+
+  gcc_jit_rvalue *val = gcc_jit_context_new_rvalue_from_int (ctxt,
+    char_type, -2);
+  gcc_jit_rvalue *return_value = gcc_jit_context_new_cast (
+    ctxt, NULL, val, int_type);
+
+  gcc_jit_block_end_with_return (block, NULL, return_value);
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+  CHECK_NON_NULL (result);
+
+  typedef int (*fn_type) ();
+  fn_type test_signed_char =
+    (fn_type)gcc_jit_result_get_code (result, "test_signed_char");
+  CHECK_NON_NULL (test_signed_char);
+  CHECK_VALUE (test_signed_char (), -2);
+}
-- 
2.43.0