On Thu, 2024-02-22 at 15:29 -0500, Antoni Boucher wrote: > Thanks for the review and idea.
Thanks for the updated patch; sorry about the delay in reviewing. > > 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? The test looks good to me, I don't think it needs a target filtering directive since presumably any target on which it already passed without the patch will still pass with the patch. The patch is OK for trunk; thanks. Dave > > 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 > > >
