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

Reply via email to