RISC-V: Fix round_32.c test on RV32

RISC-V: Fix round_32.c test on RV32

[Jivan Hakobyan]

[-- Attachment #1.1: Type: text/plain, Size: 352 bytes --]

After 8367c996e55b2 commit several checks on round_32.c test started to
fail.
The reason is that we prevent rounding DF->SI->DF on RV32 and instead of
a conversation sequence we get calls to appropriate library functions.


gcc/testsuite/ChangeLog:
        * testsuite/gcc.target/riscv/round_32.c: Fixed test


-- 
With the best regards
Jivan Hakobyan

[-- Attachment #2: rv32_round_test_fix.diff --]
[-- Type: text/x-patch, Size: 1897 bytes --]

diff --git a/gcc/testsuite/gcc.target/riscv/round_32.c b/gcc/testsuite/gcc.target/riscv/round_32.c
index 88ff77aff2e..b74be4e1103 100644
--- a/gcc/testsuite/gcc.target/riscv/round_32.c
+++ b/gcc/testsuite/gcc.target/riscv/round_32.c
@@ -7,17 +7,17 @@
 
 /* { dg-final { scan-assembler-times {\mfcvt.w.s} 15 } } */
 /* { dg-final { scan-assembler-times {\mfcvt.s.w} 5 } } */
-/* { dg-final { scan-assembler-times {\mfcvt.d.w} 65 } } */
-/* { dg-final { scan-assembler-times {\mfcvt.w.d} 15 } } */
-/* { dg-final { scan-assembler-times {,rup} 6 } } */
-/* { dg-final { scan-assembler-times {,rmm} 6 } } */
-/* { dg-final { scan-assembler-times {,rdn} 6 } } */
-/* { dg-final { scan-assembler-times {,rtz} 6 } } */
+/* { dg-final { scan-assembler-times {\mfcvt.d.w} 60 } } */
+/* { dg-final { scan-assembler-times {\mfcvt.w.d} 10 } } */
+/* { dg-final { scan-assembler-times {,rup} 5 } } */
+/* { dg-final { scan-assembler-times {,rmm} 5 } } */
+/* { dg-final { scan-assembler-times {,rdn} 5 } } */
+/* { dg-final { scan-assembler-times {,rtz} 5 } } */
 /* { dg-final { scan-assembler-not {\mfcvt.l.d} } } */
 /* { dg-final { scan-assembler-not {\mfcvt.d.l} } } */
-/* { dg-final { scan-assembler-not "\\sceil\\s" } } */
-/* { dg-final { scan-assembler-not "\\sfloor\\s" } } */
-/* { dg-final { scan-assembler-not "\\sround\\s" } } */
-/* { dg-final { scan-assembler-not "\\snearbyint\\s" } } */
-/* { dg-final { scan-assembler-not "\\srint\\s" } } */
-/* { dg-final { scan-assembler-not "\\stail\\s" } } */
+/* { dg-final { scan-assembler-times "\tceil\\s" 1 } } */
+/* { dg-final { scan-assembler-times "\tfloor\\s" 1 } } */
+/* { dg-final { scan-assembler-times "\tround\\s" 1 } } */
+/* { dg-final { scan-assembler-times "\tnearbyint\\s" 1 } } */
+/* { dg-final { scan-assembler-times "\ttrunc\\s" 1 } } */
+/* { dg-final { scan-assembler-times "\\stail\\s" 5 { target { no-opts "-O1" } } } } */

Re: RISC-V: Fix round_32.c test on RV32

[Jeff Law]

On 5/22/24 6:47 AM, Jivan Hakobyan wrote:
> After 8367c996e55b2 commit several checks on round_32.c test started to 
> fail.
> The reason is that we prevent rounding DF->SI->DF on RV32 and instead of
> a conversation sequence we get calls to appropriate library functions.
> 
> 
> gcc/testsuite/ChangeLog:
>          * testsuite/gcc.target/riscv/round_32.c: Fixed test
I wonder if this test even makes sense for rv32 anymore given we can't 
do a DF->DI as a single instruction and DF->SI is going to give 
incorrect results.  So the underlying optimization to improve those 
rounding cases just doesn't apply to DF mode objects for rv32.

Thoughts?
Jeff

Re: RISC-V: Fix round_32.c test on RV32

[Palmer Dabbelt]

On Wed, 22 May 2024 11:01:16 PDT (-0700), jeffreyalaw@gmail.com wrote:
>
>
> On 5/22/24 6:47 AM, Jivan Hakobyan wrote:
>> After 8367c996e55b2 commit several checks on round_32.c test started to
>> fail.
>> The reason is that we prevent rounding DF->SI->DF on RV32 and instead of
>> a conversation sequence we get calls to appropriate library functions.
>>
>>
>> gcc/testsuite/ChangeLog:
>>          * testsuite/gcc.target/riscv/round_32.c: Fixed test
> I wonder if this test even makes sense for rv32 anymore given we can't
> do a DF->DI as a single instruction and DF->SI is going to give
> incorrect results.  So the underlying optimization to improve those
> rounding cases just doesn't apply to DF mode objects for rv32.
>
> Thoughts?

Unless I'm missing something, we should still be able to do the 
float roundings on rv32?

I think with Zfa we'd also have testable sequences for the double/double 
and float/float roundings, which could be useful to test.  I'm not 
entirely sure there, though, as I always get a bit lost in which FP 
rounding flavors map down.

I'd also kicked off some run trying to promote these to executable 
tests.   IIRC it was just DG stuff (maybe just adding a `dg-do run`?) 
but I don't know where I stashed the results...

> Jeff

Re: RISC-V: Fix round_32.c test on RV32

[Jeff Law]

On 5/22/24 12:15 PM, Palmer Dabbelt wrote:
> On Wed, 22 May 2024 11:01:16 PDT (-0700), jeffreyalaw@gmail.com wrote:
>>
>>
>> On 5/22/24 6:47 AM, Jivan Hakobyan wrote:
>>> After 8367c996e55b2 commit several checks on round_32.c test started to
>>> fail.
>>> The reason is that we prevent rounding DF->SI->DF on RV32 and instead of
>>> a conversation sequence we get calls to appropriate library functions.
>>>
>>>
>>> gcc/testsuite/ChangeLog:
>>>          * testsuite/gcc.target/riscv/round_32.c: Fixed test
>> I wonder if this test even makes sense for rv32 anymore given we can't
>> do a DF->DI as a single instruction and DF->SI is going to give
>> incorrect results.  So the underlying optimization to improve those
>> rounding cases just doesn't apply to DF mode objects for rv32.
>>
>> Thoughts?
> 
> Unless I'm missing something, we should still be able to do the float 
> roundings on rv32?
I initially thought that as well.  The problem is we don't have a DF->DI 
conversion instruction for rv32.  We can't use DF->SI as the range of 
representable values is wrong.


> 
> I think with Zfa we'd also have testable sequences for the double/double 
> and float/float roundings, which could be useful to test.  I'm not 
> entirely sure there, though, as I always get a bit lost in which FP 
> rounding flavors map down.
Zfa is a different story as it has instructions with the proper 
semantics ;-)  We'd just emit those new instructions and wouldn't have 
to worry about the initial range test.


> 
> I'd also kicked off some run trying to promote these to executable 
> tests.   IIRC it was just DG stuff (maybe just adding a `dg-do run`?) 
> but I don't know where I stashed the results...
Not a bad idea, particularly if we test the border cases.

jeff

Re: RISC-V: Fix round_32.c test on RV32

[Palmer Dabbelt]

On Wed, 22 May 2024 12:02:26 PDT (-0700), jeffreyalaw@gmail.com wrote:
>
>
> On 5/22/24 12:15 PM, Palmer Dabbelt wrote:
>> On Wed, 22 May 2024 11:01:16 PDT (-0700), jeffreyalaw@gmail.com wrote:
>>>
>>>
>>> On 5/22/24 6:47 AM, Jivan Hakobyan wrote:
>>>> After 8367c996e55b2 commit several checks on round_32.c test started to
>>>> fail.
>>>> The reason is that we prevent rounding DF->SI->DF on RV32 and instead of
>>>> a conversation sequence we get calls to appropriate library functions.
>>>>
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>          * testsuite/gcc.target/riscv/round_32.c: Fixed test
>>> I wonder if this test even makes sense for rv32 anymore given we can't
>>> do a DF->DI as a single instruction and DF->SI is going to give
>>> incorrect results.  So the underlying optimization to improve those
>>> rounding cases just doesn't apply to DF mode objects for rv32.
>>>
>>> Thoughts?
>>
>> Unless I'm missing something, we should still be able to do the float
>> roundings on rv32?
> I initially thought that as well.  The problem is we don't have a DF->DI
> conversion instruction for rv32.  We can't use DF->SI as the range of
> representable values is wrong.

Ya, right.  I guess we'd need to be calling roundf(), not round(), for 
those?  So maybe we should adjust the tests to do that?

>> I think with Zfa we'd also have testable sequences for the double/double
>> and float/float roundings, which could be useful to test.  I'm not
>> entirely sure there, though, as I always get a bit lost in which FP
>> rounding flavors map down.
> Zfa is a different story as it has instructions with the proper
> semantics ;-)  We'd just emit those new instructions and wouldn't have
> to worry about the initial range test.

and I guess that'd just be an entirely different set of scan-assembly 
sets than round_32 or round_64, so maybe it's not a reason to keep these 
around.

>> I'd also kicked off some run trying to promote these to executable
>> tests.   IIRC it was just DG stuff (maybe just adding a `dg-do run`?)
>> but I don't know where I stashed the results...
> Not a bad idea, particularly if we test the border cases.

Ya, makes sense -- I guess the current values aren't that exciting for 
execution, but we could just add some more interesting ones...

> jeff

Re: RISC-V: Fix round_32.c test on RV32

[Jivan Hakobyan]

[-- Attachment #1: Type: text/plain, Size: 3126 bytes --]

>
> Ya, makes sense -- I guess the current values aren't that exciting for
> execution, but we could just add some more interesting ones...


During the development of the patch, I have an issue with large numbers
(2e34, -2e34).
They are used in gfortran.fortran-torture/execute/intrinsic_aint_anint.f90
test.
Besides that, a benchmark from Spec 2017 also failed (can not remember
which one),
Now we haven't an issue with them,
Of course, I can add additional tests with large numbers. But it will be
double-check (first fortran's test)


On Wed, May 22, 2024 at 11:19 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:

> On Wed, 22 May 2024 12:02:26 PDT (-0700), jeffreyalaw@gmail.com wrote:
> >
> >
> > On 5/22/24 12:15 PM, Palmer Dabbelt wrote:
> >> On Wed, 22 May 2024 11:01:16 PDT (-0700), jeffreyalaw@gmail.com wrote:
> >>>
> >>>
> >>> On 5/22/24 6:47 AM, Jivan Hakobyan wrote:
> >>>> After 8367c996e55b2 commit several checks on round_32.c test started
> to
> >>>> fail.
> >>>> The reason is that we prevent rounding DF->SI->DF on RV32 and instead
> of
> >>>> a conversation sequence we get calls to appropriate library functions.
> >>>>
> >>>>
> >>>> gcc/testsuite/ChangeLog:
> >>>>          * testsuite/gcc.target/riscv/round_32.c: Fixed test
> >>> I wonder if this test even makes sense for rv32 anymore given we can't
> >>> do a DF->DI as a single instruction and DF->SI is going to give
> >>> incorrect results.  So the underlying optimization to improve those
> >>> rounding cases just doesn't apply to DF mode objects for rv32.
> >>>
> >>> Thoughts?
> >>
> >> Unless I'm missing something, we should still be able to do the float
> >> roundings on rv32?
> > I initially thought that as well.  The problem is we don't have a DF->DI
> > conversion instruction for rv32.  We can't use DF->SI as the range of
> > representable values is wrong.
>
> Ya, right.  I guess we'd need to be calling roundf(), not round(), for
> those?  So maybe we should adjust the tests to do that?
>
> >> I think with Zfa we'd also have testable sequences for the double/double
> >> and float/float roundings, which could be useful to test.  I'm not
> >> entirely sure there, though, as I always get a bit lost in which FP
> >> rounding flavors map down.
> > Zfa is a different story as it has instructions with the proper
> > semantics ;-)  We'd just emit those new instructions and wouldn't have
> > to worry about the initial range test.
>
> and I guess that'd just be an entirely different set of scan-assembly
> sets than round_32 or round_64, so maybe it's not a reason to keep these
> around.
>
> >> I'd also kicked off some run trying to promote these to executable
> >> tests.   IIRC it was just DG stuff (maybe just adding a `dg-do run`?)
> >> but I don't know where I stashed the results...
> > Not a bad idea, particularly if we test the border cases.
>
> Ya, makes sense -- I guess the current values aren't that exciting for
> execution, but we could just add some more interesting ones...
>
> > jeff
>


-- 
With the best regards
Jivan Hakobyan

Re: RISC-V: Fix round_32.c test on RV32

[Jeff Law]

On 5/27/24 4:17 PM, Jivan Hakobyan wrote:
>     Ya, makes sense -- I guess the current values aren't that exciting for
>     execution, but we could just add some more interesting ones...
> 
> 
> During the development of the patch, I have an issue with large
> numbers (2e34, -2e34). They are used in gfortran.fortran-torture/
> execute/ intrinsic_aint_anint.f90 test. Besides that, a benchmark
> from Spec 2017 also failed (can not remember which one), Now we
> haven't an issue with them, Of course, I can add additional tests
> with large numbers. But it will be double-check (first fortran's
> test)
So i think the question is what do we want to do in the immediate term.

We can remove the test to get cleaner testresults on rv32.  I'm not a 
big fan of removing tests, but this test just doesn't make sense on rv32 
as-is.


We could leave things alone for now on the assumption the test will be 
rewritten to check for calls to the proper routines and possibly 
extended to include runtime verification.

I tend to lean towards the first.  That obviously wouldn't close the 
door on re-adding the test later with runtime verification and such.

Palmer, do you have a strong opinion either way?

jeff