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

[Xi Ruoyao <xry111@xry111.site>]

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

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