> -----Original Message----- > From: Torbjorn SVENSSON <torbjorn.svens...@foss.st.com> > Sent: Wednesday, August 21, 2024 2:23 PM > To: Tamar Christina <tamar.christ...@arm.com>; Richard Biener > <richard.guent...@gmail.com> > Cc: Jeff Law <jeffreya...@gmail.com>; gcc-patches@gcc.gnu.org; Richard > Earnshaw <richard.earns...@arm.com>; quic_apin...@quicinc.com; > yvan.r...@foss.st.com > Subject: Re: [PATCH] testsuite: Add -fwrapv to signbit-5.c > > > > On 2024-08-20 14:37, Tamar Christina wrote: > >> -----Original Message----- > >> From: Richard Biener <richard.guent...@gmail.com> > >> Sent: Tuesday, August 20, 2024 12:33 PM > >> To: Torbjorn SVENSSON <torbjorn.svens...@foss.st.com> > >> Cc: Jeff Law <jeffreya...@gmail.com>; gcc-patches@gcc.gnu.org; Richard > >> Earnshaw <richard.earns...@arm.com>; quic_apin...@quicinc.com; > >> yvan.r...@foss.st.com; Tamar Christina <tamar.christ...@arm.com> > >> Subject: Re: [PATCH] testsuite: Add -fwrapv to signbit-5.c > >> > >> On Fri, Aug 16, 2024 at 4:30 PM Torbjorn SVENSSON > >> <torbjorn.svens...@foss.st.com> wrote: > >>> > >>> > >>> > >>> On 2024-08-16 16:07, Jeff Law wrote: > >>>> > >>>> > >>>> On 8/16/24 4:12 AM, Torbjörn SVENSSON wrote: > >>>>> Ok for trunk and releases/gcc-14? > >>>>> > >>>>> Verified this on x86_64 and arm-none-eabi. > >>>>> Don't know if the other "truth type" dg-lines can be removed as well. > >>>>> > >>>>> -- > >>>>> > >>>>> On Cortex-M55 with MVE, the test case fails due to -INT_MAX being > >>>>> undefined. Adding -fwrapv solves the issues. > >>>>> > >>>>> Regtested on x86_64-pc-linux and arm-none-eabi for > >>>>> Cortex-M0/M3/M4/M7/M33/M55/M85/A7. > >>>>> > >>>>> gcc/testsuite/ChangeLog: > >>>>> > >>>>> * gcc.dg/signbit-5.c: Add -fwrapv and remove x86 exception. > >>>> Presumably the -x[i] when i == 0 cases? I'm a bit surprised that doing > >>>> a -INT_MIN didn't produce -INT_MIN, but it's still a bad thing to do due > >>>> to the overflow. > >>> > >>> On the Cortex-M55 with MVE, -INT_MIN will result in INT_MIN, i.e. a > >>> large negative value. The negated INT_MIN value cannot be represented > >>> using the two complement form with the same number of bits. > >> > >> Note this is what should happen everywhere. I think the testcase should > >> simply > >> avoid this special value - not sure why it was written this way. We > >> shouldn't > >> turn the test into -fwrapv only. Tamar, why did you cover negating > >> INT_MIN here? > > > > Precisely because it is the special case. while -INT_MIN is undefined > > behaviour > > unless -fwrapv it was hiding the constant from the language to test that the > > implementation defined behavior from scalar and vector matched. > > > > As you pointed out, -INT_MIN should be INT_MIN on most architectures. > > and that's what the test expects. However right shift of a negative value > > is > > Implementation defined. This likely indicates that this isn't the same for > > vector and scalar on MVE. > > > > I guess in hindsight the test is doing too much as it also is testing if > > the code > > can be vectorized, and the implementation defined behavior testing belongs > > in the backend. (this was 4 years ago, live and learn 😊). > > > >> Can we use INT_MIN + 1 instead? > > > > Yes, think that's the better solution, testing -fwrapv here doesn't seem > > right > > as signbit-4.c already tests the wrapv behavior just with scalar. And also > > using > > -fwrapv tests a different part of the code as the pattern is guarded by > > TYPE_OVERFLOW_UNDEFINED. > > Are you going to provide a patch chaining this or am I supposed to do it? > I don't mind doing it, but I have a feeling that you know better what is > expected or not here than I do. :)
Sure, I'll submit one when I'm back next week. Tamar > > Kind regards, > Torbjörn > > > > > Thanks, > > Tamar > >> > >> Richard. > >> > >>> > >>>> So, OK for the trunk and release branches. If we need to adjust risc-v > >>>> we'll know if a few days :-) > >>> > >>> Ok. > >>> > >>> > >>> Pushed as r15-2950 and r14-10592. > >>> > >>> Kind regards, > >>> Torbjörn
