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