diff mbox series

libgccjit: Support signed char flag

Message ID 455400c598a6a9e0932c4c5b15c5d8fc30355ade.camel@zoho.com
State Accepted
Headers
Series libgccjit: Support signed char flag |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Antoni Boucher Dec. 21, 2023, 1:42 p.m. UTC 
  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.

Comments

David Malcolm Jan. 9, 2024, 4:01 p.m. UTC | #1 
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
Antoni Boucher Feb. 22, 2024, 8:29 p.m. UTC | #2 
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
>
diff mbox series

Patch

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