> -----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. 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
