On 1/26/25 05:29, Jeff Law wrote:
>
> On 1/24/25 3:12 PM, Vineet Gupta wrote:
>> RV-Vector FP-INT insns use the rounding mode in FRM register which if
>> explicitly set for V insn needs, is saved/restored (although from the
>> psABI CC Spec, it is not clear if it actually a caller-saved or
>> callee-saved).
>>
>> Anyhow in the failure case the save/restore were generated by the
>> Mode Switch pass, but then eliminated by sched1:DCE and Late-Combine.
>> Fix this by using unspec_volatile variant which won't be eliminated.
>>
>> This showed up as SPEC2017 527.cam4 runtime aborts in glibc:round_away()
>> which checks for standard rounding modes and the "leaking" rounding mode
>> due to the bug happened to be a non-standard RISC-V specific RMM
>> "Round to Nearest, ties to Max".
>>
>> This is testsuite clean:
> Not sure how it could be clean as I think the test itself is busted ;-)
Sorry for the snafu; I would blame "send the patch out on friday" that bit me.
I used the raw test for debugging and coming up with the fix, but before
integrating the test properly I ran the existing testsuite w/ my patch and made
sure it didn't regress anything, so those test results are for real minus the
new test.
> As-is it'll trigger compile time failures:
>> FAIL: gfortran.target/riscv/rvv/pr118646.f90 -O0 (test for excess errors)
>> Excess errors:
>> /home/jlaw/test/gcc/gcc/testsuite/gfortran.target/riscv/rvv/pr118646.f90:18:12:
>> Warning: Deleted feature: End expression in DO loop at (1) must be integer
>> /home/jlaw/test/gcc/gcc/testsuite/gfortran.target/riscv/rvv/pr118646.f90:22:15:
>> Warning: Deleted feature: End expression in DO loop at (1) must be integer
>> /home/jlaw/test/gcc/gcc/testsuite/gfortran.target/riscv/rvv/pr118646.f90:36:18:
>> Warning: Deleted feature: End expression in DO loop at (1) must be integer
Added -std-leagcy to address that
> A "-w" will work around that, but then there's no Fortran equivalent of
> main and we get a link error:
>> FAIL: gfortran.target/riscv/rvv/pr118646.f90 -O0 (test for excess errors)
>> Excess errors:
>> /release/linux/.build/src/glibc-git-4d29ec7c/csu/../sysdeps/riscv/start.S:67:(.text+0x22):
>> undefined reference to `main'
>>
>> UNRESOLVED: gfortran.target/riscv/rvv/pr118646.f90 -O0 compilation failed
>> to produce executable
Its not a run test, we just have to scan number of f.rm instances, which was
also missing in the test :-(
> What I wanted to do was use your testcase with Pan's patch to see if
> Pan's patch resolved both issues.
Yes it does.
> Your compiler patch may still be desirable as well, I really haven't
> really evaluated that yet.
FWIW with Pan's rework the need for volatile goes away anyways.
The fixed up test (actually tested for A/B pass/fail) which can be integrated in
Pan's commit can be found below
--------->
diff --git a/gcc/testsuite/gfortran.target/riscv/rvv/pr118646.f90
b/gcc/testsuite/gfortran.target/riscv/rvv/pr118646.f90
new file mode 100644
index 000000000000..e2a034316123
--- /dev/null
+++ b/gcc/testsuite/gfortran.target/riscv/rvv/pr118646.f90
@@ -0,0 +1,48 @@
+! Reduced from SPEC2017 527.cam4 zm_conv.F90
+
+! { dg-do compile }
+! { dg-options "-Ofast -std=legacy -march=rv64gcv_zvl256b_zba_zbb_zbs_zicond
-ftree-vectorize -mabi=lp64d" }
+
+module a
+ contains
+subroutine b(f)
+
+ real d(4)
+ integer e(4)
+ integer f(4)
+ real hmax(4)
+ real g(4)
+
+ integer h(4)
+ integer l(4,5)
+ do i = 1,c
+ h(i) = 0
+ end do
+ do k = j ,1
+ do i = 1,c
+ q = g(i) + hmax(i)
+ if (k >= nint(d(i)) .and. k <= e(i) .and. q > 1.e4) then
+ f(i) = k
+ end if
+ if (k < o ) then
+ if (buoy<= 0.) then
+ l(i,h) = k
+ end if
+ end if
+ end do
+ end do
+ do n = 1,5
+ do k = 1,m
+ do i = 1,c
+ if (k > l(i,n)) then
+ p = r()
+ end if
+ end do
+ end do
+ end do
+end
+end
+
+! { dg-final { scan-assembler-times {frrm\s+[axs][0-9]+} 1 } }
+! { dg-final { scan-assembler-times {fsrmi\s+[01234]} 1 } }
+! { dg-final { scan-assembler-times {fsrm\s+[axs][0-9]+} 1 } }