On 2/7/22 10:58, Palmer Dabbelt wrote:
On Mon, 07 Feb 2022 09:41:10 PST (-0800), Vineet Gupta wrote:


On 2/7/22 01:28, Philipp Tomsich wrote:
Vineet,

On Mon, 7 Feb 2022 at 07:06, Vineet Gupta <vine...@rivosinc.com> wrote:
This is at par with other major arches such as aarch64, i386, s390 ...

No testsuite regressions: same numbers w/ w/o
Putting that check seems a good idea, but I haven't seen any cases
related to this get through anyway.
Do you have seen any instances where the backend got this wrong? If
so, please share, so we can run a fuller regression and see any
performance impact.

No, there were no failures which this fixes. Seems like other arches did
this back in 2015.
When aarch64 did similar change, commit 2ca5b4303bd5, a directed generic
test pr68129_1.c was added which doesn't fail before/after.

The only offending MD pattern we had was for for constant 0, which IIUC should be a const_int now (and has been for some time) so shouldn't even have been matching anything.  I was worried about the fcvt-based moves on rv32, but my trivial test indicates those still work fine

   double foo(void)
   {
       return 0;
   }
      foo:
       fcvt.d.w    fa0,x0
       ret

so I'm assuming they're coming in through const_int as well. Probably worth a full rv32 testsuite run, but as far as I can tell we were essentially TARGET_SUPPORTS_WIDE_INT clean already so this should be pretty safe.

Ok I'll go off and run the rv32 suite just to be safe.


Unfortunately the patch isn't trivially applying on trunk: it's targeting the wrong files and is showing some whitespace issues (though those may have been a result of me attempting to clean stuff up).  I assuming that means that the tests weren't run on trunk, though.

I tested both gcc 11 and trunk. Both were clean. My bad that I posted the patch off of internal gcc 11 tree.


I put a cleaned up version over here <https://github.com/palmer-dabbelt/gcc/commit/1df538132d45d6d80bdb5d2dbee4d7d33606e6da.patch> in case that helps anyone.  I haven't run the regressions, but otherwise

Reviewed-by: Palmer Dabbelt <pal...@rivosinc.com>

LMK if you want me to run the test suite.

I'd be great if you can verify. I'll go off and setup a rc32 test setup as well.

IIUC we're still a bit away from the GCC 12 branch, and given this doesn't fix any manifestable bugs it should be held for 13.

Sure thing.

Thx,
-Vineet

Reply via email to