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