Re: [PATCH v2] LoongArch: testsuite:Added additional vectorization "-mlsx" option.

[Xi Ruoyao <xry111@xry111.site>]

On Tue, 2024-01-16 at 12:58 +0800, Xi Ruoyao wrote:
> On Tue, 2024-01-16 at 10:57 +0800, chenxiaolong wrote:
> > 在 2024-01-15一的 15:50 +0800，Xi Ruoyao写道：
> > > On Mon, 2024-01-15 at 15:10 +0800, chenxiaolong wrote:
> > > > At 14:42 +0800 on the first day of 2024-01-15, Xi Ruoyao wrote：
> > > > > On Mon, 2024-01-15 at 14:32 +0800, YunQiang Su wrote:
> > > > > > Xi Ruoyao <xry111@xry111.site> wrote at 12:11pm on Monday,
> > > > > > January
> > > > > > 15, 2024：
> > > > > > > On Mon, 2024-01-15 at 09:29 +0800, chenxiaolong wrote:
> > > > > > > > At 21:13 +0800 on Saturday, 2024-01-13, Xi Ruoyao wrote：
> > > > > > > > > At 15:28 +0800 on Saturday 2024-01-13, chenxiaolong
> > > > > > > > > wrote：
> > > > > > > > > > gcc/testsuite/ChangeLog:
> > > > > > > > > > 
> > > > > > > > > >     * gcc.dg/pr104992.c: Added additional "-mlsx"
> > > > > > > > > > compilation
> > > > > > > > > > options.
> > > > > > > > > >     * gcc.dg/signbit-2.c: Dito.
> > > > > > > > > >     * gcc.dg/tree-ssa/scev-16.c: Dito.
> > > > > > > > > >     * gfortran.dg/graphite/vect-pr40979.f90: Dito.
> > > > > > > > > >     * gfortran.dg/vect/fast-math-mgrid-resid.f: Dito.
> > > > > > > > > 
> > > > > > > > > I don't feel it right about the changes to pr104992.c and
> > > > > > > > > scev-16.c
> > > > > > > > > because no other architectures add special options
> > > > > > > > > there. 
> > > > > > > > > Why are we
> > > > > > > > > so special?
> > > > > > > > Because on the LoongArch architecture, GCC requires the
> > > > > > > > addition of
> > > > > > > > vectorization options in order to generate vector code. Use
> > > > > > > > the
> > > > > > > > check_effective_target_vect_cmdline_needed command in the
> > > > > > > > lib/target-
> > > > > > > > supports.exp file to set whether the command line option is
> > > > > > > > needed to
> > > > > > > > enable vectorizations. For example, ia64,x86,aarch64, and
> > > > > > > > riscv
> > > > > > > > architectures, vectorization is enabled by default.
> > > > > > > 
> > > > > > > But no.  The default baseline of 32-bit x86 is i686, which is
> > > > > > > basically
> > > > > > > a Pentium III launched in 1999 without any vector
> > > > > > > instructions.
> > > > > > > 
> > > > > > > We are still missing something here.
> > > > > > > 
> > > > > > There is a line
> > > > > >        #define vector
> > > > > > __attribute__((vector_size(4*sizeof(int))))
> > > > > > I guess it is the syntax needs to be supported.
> > > > > 
> > > > > This is always supported.  If the target does not have vector
> > > > > instructions GCC will just expand vector arithmetic as a loop.
> > > > > 
> > > > > Maybe we should just move this test into gcc.dg/vect where the
> > > > > framework
> > > > > automatically add options like -mlsx or -msse2?
> > > > > 
> > > > 
> > > > The "-mlsx" option is turned on by default after vectorization
> > > > testing
> > > > is turned on. However, the use of dg-options in some files resets
> > > > the
> > > > compilation options for testing this file. Therefore, to detect
> > > > vectorization on LoongArch, it is necessary to add an additional "-
> > > > mlsx" option.
> > > 
> > > Then it should use dg-additional-options instead of dg-options.
> > > 
> > According to your advice, I have tried the following two ways:
> > 
> > (1)Replace dg-options directly with dg-additional-options. The "-ansi-
> > pedantic-errors" set in the dg.exp file is used, and the following
> > problems occur:
> > 
> > gcc.dg/pr104992.c:ISO C90 does not support complex types.
> > gcc.dg/tree-ssa/scev-16.c:‘for’ loop initial declarations are only
> > allowed in C99 or C11 mode
> > 
> > Note: The ISO required by the program is inconsistent with the default
> > standard, resulting in an error.
> 
> We can add -std=c11 into dg-additional-options as well.
> 
> > (2)Move pr104992.c and scev-16.c to the gcc.dg/vect directory and
> > replace dg-options with dg-additional-options. The problems are as
> > follows:
> > 
> > gcc.dg/vect/scev-16.c: Because there is no test rule starting with
> > scev* in the vect.exp file, you need to add a new test rule or change
> > the file name before the test can be performed.
> > 
> > Summary: It is more appropriate to add the additional "-mlsx" option
> > directly to the pr104992.c and scev-16.c files. This supports
> > vectorization  testing of the LoongArch architecture and does not
> > modify the testing behavior of other architectures.
> 
> No, "not modifying the other architectures" may not be a good thing in
> some cases.
> 
> There are only two possibilities here:
> 
> (1) The test is broken for all architectures where vectorization needs
> additional options.  Then we should fix it for all the architectures.
> 
> (2) The test is only broken on LoongArch, then on other architectures
> some mechanism is already adding the proper options for vectorization.
> Then we should implement the same mechanism for LoongArch.
> 
> For either case adding -mlsx only for LoongArch is only papering over
> the real issue.
> 
> I'll run these tests on some other architectures and see what's
> happening...

Alright, now to me using vect_* out of vect directories is simply wrong.
Instead of fixing up all the tests one by one, target by target, we
should just stop this nonsense.  Spreading target-specific vectorization
options everywhere is pretty unmaintainable.

https://gcc.gnu.org/PR113418

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University