Re: [PATCH v1 1/6] LoongArch: a symmetric multilib subdir layout

2023-08-14 Thread Yujie Yang
On Mon, Aug 14, 2023 at 01:38:40PM +0800, Xi Ruoyao wrote:
> On Mon, 2023-08-14 at 11:57 +0800, Yang Yujie wrote:
> 
> > However, for LoongArch, we do not want such a "toplevel" library
> > installation since the default ABI may change.  We expect all
> > multilib variants of libraries to be installed to their designated
> > ABI-specific subdirs (e.g. base/lp64d) of the GCC libdir, so that
> > the default ABI can be configured arbitrarily (with --with-abi)
> > while the gcc libdir layout stays consistent.  This could be
> > helpful for the distribution packaging of GCC libraries.
> 
> Have you tested a --disable-multilib configuration?  To me with --
> disable-configuration everything should be still in the toplevel
> directory, not any sub-directory.

That's a good point, sorry I missed --disable-multilib here.

However, you don't really need --disable-multilib since
the libraries are only built once in the default ABI configuration
as long as --with-multilib-list does not request anything more than
that.

Maybe we should force-enabling multilib in all cases.



Re: [PATCH v1 2/6] LoongArch: improved target configuration interface

2023-08-14 Thread Yujie Yang
On Mon, Aug 14, 2023 at 01:58:24PM +0800, Xi Ruoyao wrote:
> On Mon, 2023-08-14 at 11:57 +0800, Yang Yujie wrote:
> > * Support options for LoongArch SIMD extensions:
> >   new configure options --with-simd={none,lsx,lasx};
> >   new driver options -m[no]-l[a]sx / -msimd={none,lsx,lasx}.
> 
> I suggest to rename --with-simd= to --with-ext= and accept a comma-
> separated ISA extension list, because we have non-SIMD ISA extensions. 
> For example, "--with-ext=lasx,lbt" will make -mlasx, -mlsx (implied),
> and -mlbt the default.  I prefer "-mlasx" over "-msimd=lasx" because "-
> mlasx" is shorter anyway (if there is no real reason to make -mlasx and
> -msimd=lasx two different things).
> 
> -- 
> Xi Ruoyao 
> School of Aerospace Science and Technology, Xidian University

Thanks for the suggestion, --with-ext seems a good idea.  I will
consider adding it later.

-msimd=lasx and -mlasx are the same if you only use them once, but things
gets complicated if you also want -mno-lsx to cancel the effect of -mlasx.

In short, -msimd= is *necessary* for a correct (and convenient) implementation
of the "legacy" -m[no-]l[a]sx options.  Let's hope I can explain this clearly:

I assume we all want:

 (1) -mlasx -mlsx -> enable LSX and LASX
 (2) -mlasx -mno-lsx -> disable LSX and LASX
 (3) -mno-lsx -mlasx -> enable LSX and LASX

Unless we declare -mlsx / -mlasx as driver deferred, AFAIK there is no other 
way for
us to know the actual order of appearnce of all -m[no-]l[a]sx options on the 
command
line.  All we know from GCC's option system would be a final on/off state of 
"lsx"
and a final on/off state of "lasx".

These states results from independent processing of -m[no-]lsx and -m[no-]lasx 
options
in the order they appear, respectively.  So we can't distinguish between (2) 
and (3).

Another seemingly viable approach is to declare three options -mlsx / -mlasx / 
-mno-lsx
and making them mutually exclusive.  In this way GCC would only consider the 
last one
appearing on the command line.  But we lost (1) in this case.

So, it seems that GCC is forbidding a compiler option to express a "state 
change"
rather than a independent configuration state itself.  This makes sense since
"-grecord-gcc-switches" and the LTO objects prefers that a target configuration
can be uniquely expressed in a "tuple of coordinates", rather than a series of
"connected vectors", so that they can be easily compared.

Now what we want is clear:

1. support convenient options like -mlsx / -mlasx (maybe something like
-m32 / -m64 in the future) that express state changes that interferes with
the effect of prior flags with different names.

2. the options passed to the compiler proper has a clear one-one
correspondence to the target configuration.


The implementation: canonicalize everything in the GCC driver, process the 
"flags"
that express state changes in the order they appear into a group of independent
parameters.  If the parameters they have overlapping semantics, the priority
is hard-coded, like -march and -msimd.

(This is also necessary for multilib since this is where the multilib selection
happens and final ABI needs to be decided before that.)

For this purpose, -msimd= is the canonicalized parameter form of the state
of SIMD extension, and -m[no-]l[a]sx are defined as convenient driver-only
flags.



Re: [PATCH v1 2/6] LoongArch: improved target configuration interface

2023-08-14 Thread Yujie Yang
On Mon, Aug 14, 2023 at 04:49:11PM +0800, Xi Ruoyao wrote:
> On Mon, 2023-08-14 at 16:44 +0800, Yujie Yang wrote:
> > I assume we all want:
> > 
> >  (1) -mlasx -mlsx -> enable LSX and LASX
> >  (2) -mlasx -mno-lsx -> disable LSX and LASX
> >  (3) -mno-lsx -mlasx -> enable LSX and LASX
> 
> Yes.
> 
> > Unless we declare -mlsx / -mlasx as driver deferred, AFAIK there is no 
> > other way for
> > us to know the actual order of appearnce of all -m[no-]l[a]sx options on 
> > the command
> > line.  All we know from GCC's option system would be a final on/off state 
> > of "lsx"
> > and a final on/off state of "lasx".
> 
> But x86 does this correct;
> 
> $ echo __AVX__ + __AVX2__ | LANG= cpp -E -mno-avx -mavx2
> # 0 ""
> # 0 ""
> # 0 ""
> # 1 "/usr/include/stdc-predef.h" 1 3 4
> # 0 "" 2
> # 1 ""
> 1 + 1
> 
> so there must be a way to handle this...
> 
> -- 
> Xi Ruoyao 
> School of Aerospace Science and Technology, Xidian University

Emm... What happens if you reverse the order?

$ echo __AVX__ + __AVX2__ | LANG= cpp -E -mavx2 -mno-avx

Anyways, I believe there may be other ways to implement this, but it would
require equally much effort (or even much more) that the current approach.
Especially considering the possiblity of future updates -- we now have a
framework for this sort of things.

Meanwhile you confortably can stay away from -msimd= and use only
-mlsx / -mlasx. So...a matter of style maybe?



Re: [PATCH v1 2/6] LoongArch: improved target configuration interface

2023-08-14 Thread Yujie Yang
On Mon, Aug 14, 2023 at 01:22:32PM +0800, Xi Ruoyao wrote:
> On Mon, 2023-08-14 at 11:57 +0800, Yang Yujie wrote:
> > The configure script and the GCC driver are updated so that
> > it is easier to customize and control GCC builds for targeting
> > different LoongArch implementations.
> > 
> > * Support options for LoongArch SIMD extensions:
> >   new configure options --with-simd={none,lsx,lasx};
> >   new driver options -m[no]-l[a]sx / -msimd={none,lsx,lasx}.
> 
> What's the relationship between -mlasx and -msimd=lasx?  What will
> happen if the user specifies -mlasx -msimd=none or -mlasx -msimd=lsx?
> 
> -- 
> Xi Ruoyao 
> School of Aerospace Science and Technology, Xidian University

At this moment we make sure all "flags" (that expresses a config "delta")
are processed after the "parameters", which is documented in the LoongArch
Toolchain Conventions[1].

So if -msimd=* and -m[no-]l[a]sx appear together on the driver command
line, the final configuration would be derived by first applying -msimd=*
and then the sequence of -m[no-]l[a]sx, in the order they appear.

This is similar to the relationship between -msoft-float and -mfpu / -mabi,
where -mfpu / -mabi are applied first, and -msoft-float modifies the existing
target configuration states (FPU, ABI).

[1] currently released at https://github.com/loongson/LoongArch-Documentation
/blob/main/docs/LoongArch-toolchain-conventions-EN.adoc



Re: [PATCH v1 1/6] LoongArch: a symmetric multilib subdir layout

2023-08-14 Thread Yujie Yang
On Mon, Aug 14, 2023 at 03:48:53PM +0800, Xi Ruoyao wrote:
> On Mon, 2023-08-14 at 15:37 +0800, Yujie Yang wrote:
> > On Mon, Aug 14, 2023 at 01:38:40PM +0800, Xi Ruoyao wrote:
> > > On Mon, 2023-08-14 at 11:57 +0800, Yang Yujie wrote:
> > > 
> > > > However, for LoongArch, we do not want such a "toplevel" library
> > > > installation since the default ABI may change.  We expect all
> > > > multilib variants of libraries to be installed to their designated
> > > > ABI-specific subdirs (e.g. base/lp64d) of the GCC libdir, so that
> > > > the default ABI can be configured arbitrarily (with --with-abi)
> > > > while the gcc libdir layout stays consistent.  This could be
> > > > helpful for the distribution packaging of GCC libraries.
> > > 
> > > Have you tested a --disable-multilib configuration?  To me with --
> > > disable-configuration everything should be still in the toplevel
> > > directory, not any sub-directory.
> > 
> > That's a good point, sorry I missed --disable-multilib here.
> > 
> > However, you don't really need --disable-multilib since
> > the libraries are only built once in the default ABI configuration
> > as long as --with-multilib-list does not request anything more than
> > that.
> > 
> > Maybe we should force-enabling multilib in all cases.
> 
> I really don't like this.  Why must I always remind my self "hey, this
> is LoongArch, there is a different directory layout" when I don't need
> multilib at all?
> 

AFAIK, the two main uses of the multisubdir layout are in the C++
header directory and the GCC libdir (where libgcc.a resides), respectively.
The GCC libdir is fine since they are private to a user's GCC build.
However, the C++ header directory is shared across the system unless
an alternative sysroot is chosen, so the consisentency of the multilib
layout matters.

So theoretically, the toplevel libraries should have the same ABI under
the the target triplet.  However, for many architectures, the
"--with-abi + MULTILIB_DEFAULT" scheme may cause the toplevel to be
configured to have different meanings.

So I think it's also a reasonable approach that we just simply eliminate
the ambiguous toplevel libraries and use a symmetric layout instead.



Re: [PATCH v1 1/6] LoongArch: a symmetric multilib subdir layout

2023-08-14 Thread Yujie Yang
> I came up with another idea. What if we:
> 
> 1. Keep the "default" ABI libs in the toplevel directory. There is
> *always* a default ABI so treating it specially is not really nonsense.
> 2. Create a symlink for consistency. For example, if --with-abi=lp64d, -
> -with-multilib-list=lp64d,lp64s:
> 
>  * /usr/lib/gcc/loongarch64-linux-gnu/14.0.0 contains the lp64d
>libraries.
>  * /usr/lib/gcc/loongarch64-linux-gnu/14.0.0/lp64s contains the lp64s
>libraries.
>  * /usr/lib/gcc/loongarch64-linux-gnu/14.0.0/lp64d is a symlink to "."
> 
> Then we can refer to the lp64d libgcc.a with both
> /usr/lib/gcc/loongarch64-linux-gnu/14.0.0/lp64d/libgcc.a, and
> /usr/lib/gcc/loongarch64-linux-gnu/14.0.0/libgcc.a.
> 
> For referring to the default multilib, the non-suffixed
> /usr/lib/gcc/loongarch64-linux-gnu/14.0.0 path should be used; for
> referring lp64d (no matter what the default is),
> /usr/lib/gcc/loongarch64-linux-gnu/14.0.0/lp64d should be used.
> 
> The symlink can be created by the GCC building system or manually by the
> distro maintainer (or gcc packager).
> 
> Thoughts?
> 
> -- 
> Xi Ruoyao  School of Aerospace Science and
> Technology, Xidian University

Yes, this also eliminates the duplicate build.

In this case, the symlink would not really be necessary since the
toplevel directory is searched by the driver for all ABI configurations
anyways.

It is even easier to implement:

1. (gcc/config.gcc) stipulate that
   loongarch64-linux-gnu== --with-abi=lp64d,
   loongarch64-linux-gnuf64 == --with-abi=lp64d,
   loongarch64-linux-gnuf32 == --with-abi=lp64f,
   loongarch64-linux-gnusf  == --with-abi=lp64s,
   and no customization is allowed.
   (maybe we can simply remove --with-abi?)

2. (config-ml.in) delete the "default" multisubdir from ${multidirs}.
   (which is base/lp64d for --with-abi=lp64d)

No other tweaking in config-ml.in is required.  So this seems to be
canonical.

The only problem is, I understand that triplets are essential to
the GNU build system, but should they always imply the default
ABI to be used when the compiler is invoked without an argument?



Re: [PATCH] LoongArch: Fix multiarch tuple canonization

2023-02-15 Thread Yujie Yang
On Tue, Feb 14, 2023 at 11:32:00AM +0800, Lulu Cheng wrote:
> add yangyujie.
 
Looks good to me. Thanks for the forward!



Re: [PATCH] LoongArch: Stop -mfpu from silently breaking ABI

2023-03-02 Thread Yujie Yang
On Fri, Mar 03, 2023 at 12:01:22AM +0800, Xi Ruoyao via Gcc-patches wrote:
> But then it causes "-mabi=lp64s -march=loongarch64" to generate code like:
> 
>   movgr2fr.d $fa0, $a0
>   frecip.d   $fa0, $fa0
>   movfr2gr.d $a0, $fa0
> 
> The problem here is "loongarch64" is never strictly defined.  So we
> consider "loongarch64" a "64-bit LoongArch CPU with the simplest FPU
> needed by the ABI", and if -march=loongarch64 but -mfpu is not
> explicitly used, we set -mfpu such a simplest one.

Thanks for the fix on TARGET_*_FLOAT_ABI usage! Certainly more testing
needs to be done on the soft-float side.

However, "loongarch64" is defined to include the "fpu64" ISA module[1]
(i.e. enable "-mfpu=64" when -mfpu is not explicitly used). So I believe
the above behavior you observed is expected.

[1] Table 5. Target CPU Models,
https://loongson.github.io/LoongArch-Documentation/LoongArch-toolchain-conventions-EN.html



Re: [PING][PATCH] LoongArch: initial ada support on linux

2023-08-24 Thread Yujie Yang
Hi!

I'd like to ping this patch for acknowledgement from the Ada team.

We have successfully compiled a cross-native toolchain with Ada enabled
for loongarch64-linux-gnuf64 (or loongarch64-linux-gnu), and have run the
regtests with the following results:

While the failures are being worked on, we would like to merge this patch
first so we can have basic ada support for debian test-builds.

=== gnat Summary ===

# of expected passes3376
# of unexpected failures1
# of expected failures  23
# of unsupported tests  25

FAIL: gnat.dg/prot7.adb (test for excess errors)

=== acats Summary ===
# of expected passes2325
# of unexpected failures3

*** FAILURES: c35503d c35503f c4a007a

Sincerely,
Yujie



Re: [PATCH v2 3/4] LoongArch: add new configure option --with-strict-align-lib

2023-08-29 Thread Yujie Yang
> > LoongArch processors may not support memory accesses without natural
> > alignments.  Building libraries with -mstrict-align may help with
> > toolchain binary compatiblity and performance on these implementations
> > (e.g. Loongson 2K1000LA).
> > 
> > No significant performance degredation is observed on current mainstream
> > LoongArch processors when the option is enabled.
> > 
> > gcc/ChangeLog:
> > 
> > * config.gcc: use -mstrict-align for building libraries
> > if --with-strict-align-lib is given.
> 
> Isn't this equivalent to --with-default-multilib=mno-strict-align now?
> 
> And I still believe the easiest way for 2K1000LA is adding -march=la264
> support so the user can simply configure with --with-arch=la264.

Not exactly -- Options given in --with-multilib-default= will not be applied
to multilib variants that have build options specified in --with-multilib-list,
but --with-strict-align-lib is always effective.

e.g. for the following configuration:

  --with-multilib-default=mstrict-align
  --with-multilib-list=lp64d/la464,lp64s

The library build options would be:

  base/lp64d variant: -mabi=lp64d -march=la464 (no -mstrict-align appended)
  base/lp64s variant: -mabi=lp64s -march=abi-default -mstrict-align

Sure, you can do it with --with-arch=la264. It's just a convenient
switch that we can use for building generic toolchains.



Re: [PATCH v2 3/4] LoongArch: add new configure option --with-strict-align-lib

2023-08-30 Thread Yujie Yang
On Wed, Aug 30, 2023 at 04:22:13PM +0800, Xi Ruoyao wrote:
> On Wed, 2023-08-30 at 14:51 +0800, Yujie Yang wrote:
> > > > LoongArch processors may not support memory accesses without natural
> > > > alignments.  Building libraries with -mstrict-align may help with
> > > > toolchain binary compatiblity and performance on these implementations
> > > > (e.g. Loongson 2K1000LA).
> > > > 
> > > > No significant performance degredation is observed on current mainstream
> > > > LoongArch processors when the option is enabled.
> > > > 
> > > > gcc/ChangeLog:
> > > > 
> > > > * config.gcc: use -mstrict-align for building libraries
> > > > if --with-strict-align-lib is given.
> > > 
> > > Isn't this equivalent to --with-default-multilib=mno-strict-align now?
> > > 
> > > And I still believe the easiest way for 2K1000LA is adding -march=la264
> > > support so the user can simply configure with --with-arch=la264.
> > 
> > Not exactly -- Options given in --with-multilib-default= will not be applied
> > to multilib variants that have build options specified in 
> > --with-multilib-list,
> > but --with-strict-align-lib is always effective.
> > 
> > e.g. for the following configuration:
> > 
> >   --with-multilib-default=mstrict-align
> >   --with-multilib-list=lp64d/la464,lp64s
> > 
> > The library build options would be:
> > 
> >   base/lp64d variant: -mabi=lp64d -march=la464 (no -mstrict-align appended)
> >   base/lp64s variant: -mabi=lp64s -march=abi-default -mstrict-align
> > 
> > Sure, you can do it with --with-arch=la264. It's just a convenient
> > switch that we can use for building generic toolchains.
> 
> If you want a generic toolchain, it should default to -mstrict-align as
> well.  Or it will still do unexpected thing for cases like:
> 
> struct foo { char x; int y; } __attribute__ ((packed));
> 
> int get (struct foo *foo) { return foo->y; }
> 
> So it should be --with-strict-align (it should make the *compiler*
> default to -mstrict-align).  But them it seems --with-arch=la264 is just
> easier...

By "generic" I mean: when you enable "-march=la264"/"-march=la464"
and link statically, you get a binary that's good for running on
LA264/LA464 cores, respectively.  It's more of a cross-toolchain case.



Re: [PATCH v2 1/4] LoongArch: improved target configuration interface

2023-08-30 Thread Yujie Yang
On Wed, Aug 30, 2023 at 09:36:22PM +, Joseph Myers wrote:
> On Wed, 30 Aug 2023, Yang Yujie wrote:
> 
> > +A suffix @code{[/ARCH][/OPTION]...]} may follow immediately after the ABI
> > +identifier to customize the compiler options for building the given set of
> > +libraries.  @code{ARCH} denotes the architecture name recognized by the
> > +@code{-march=ARCH} compiler option, which acts as a basic target ISA
> > +configuration that can be adjusted using the subsequent @code{OPTION}
> > +suffixes, where each @code{OPTION} is a compiler option itself.
> 
> Since ARCH and OPTION are not literal strings of program source code, you 
> should actually be using @var{arch} and @var{option} for them (and @dots{} 
> instead of ..., since the ... isn't literal source code either).
> 
> This patch series also adds a new configure option --with-strict-align-lib 
> that needs documenting in the corresponding patch.
> 
> -- 
> Joseph S. Myers
> jos...@codesourcery.com

Thanks for the review.  Does the following fix look good?
If so, I will include these in the patchset.

Yujie

---
 gcc/doc/install.texi | 44 +++-
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 05a626280b7..3e589080f4e 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -1236,7 +1236,7 @@ sysv, aix.
 @itemx --without-multilib-list
 Specify what multilibs to build.  @var{list} is a comma separated list of
 values, possibly consisting of a single value.  Currently only implemented
-for aarch64*-*-*, arm*-*-*, loongarch64-*-*, riscv*-*-*, sh*-*-* and
+for aarch64*-*-*, arm*-*-*, loongarch*-*-*, riscv*-*-*, sh*-*-* and
 x86-64-*-linux*.  The accepted values and meaning for each target is given
 below.
 
@@ -1331,27 +1331,27 @@ the following ABI identifiers: @code{lp64d[/base]} 
@code{lp64f[/base]}
 @code{lp64d[/base]} (the @code{/base} suffix may be omitted)
 to enable their respective run-time libraries.
 
-A suffix @code{[/ARCH][/OPTION]...]} may follow immediately after the ABI
-identifier to customize the compiler options for building the given set of
-libraries.  @code{ARCH} denotes the architecture name recognized by the
-@code{-march=ARCH} compiler option, which acts as a basic target ISA
-configuration that can be adjusted using the subsequent @code{OPTION}
-suffixes, where each @code{OPTION} is a compiler option itself.
+A suffix @code{[/@var{arch}][/@var{option}/@dots{}]} may follow immediately
+after the ABI identifier to customize the compiler options for building the
+given set of libraries.  @var{arch} denotes the architecture name recognized
+by the @option{-march=@var{arch}} compiler option, which acts as a basic target
+ISA configuration that can be adjusted using the subsequent @var{option}
+suffixes, where each @var{option} is a compiler option itself.
 
-If none of such suffix is present, the configured value of
-@option{--with-multilib-default} can be used as a common default suffix
-for all library ABI variants.  Otherwise, the default build option
-@code{-march=abi-default} is applied when building the variants without
-a suffix.
+If no such suffix is present for a given multilib variant, the
+configured value of @code{--with-multilib-default} is appended as a default
+suffix.  If @code{--with-multilib-default} is not given, the default build
+option @code{-march=abi-default} is applied when building the variants
+without a suffix.
 
-As a special case, @code{fixed} may be used in the position of @code{ARCH},
-which means use the architecture configured with @option{--with-arch=ARCH},
-or its default value (e.g. @code{loongarch64} for @code{loongarch64-*}
-targets).
+As a special case, @code{fixed} may be used in the position of @var{arch},
+which means using the architecture configured with
+@code{--with-arch=@var{arch}}, or its default value (e.g. @code{loongarch64}
+for @code{loongarch64-*} targets).
 
-If @var{list} is empty or @code{default}, or if @option{--with-multilib-list}
-is not specified, then the default ABI as specified by @option{--with-abi} or
-implied by @option{--target}.
+If @var{list} is empty or @code{default}, or if @code{--with-multilib-list}
+is not specified, then only the default variant of the libraries are built,
+where the default ABI is implied by the configured target triplet.
 
 @item riscv*-*-*
 @var{list} is a single ABI name.  The target architecture must be either
@@ -1414,6 +1414,9 @@ Multiple @code{OPTION}s may appear consecutively while 
@code{ARCH} may only
 appear in the beginning or be omitted (which means @code{-march=abi-default}
 is applied when building the libraries).
 
+@item --with-strict-align-lib
+On LoongArch targets, build all enabled multilibs with @code{-mstrict-align}
+(Not enabled by default).
 
 @item --with-multilib-generator=@var{config}
 Specify what multilibs to build.  @var{config} is a semicolon separated list of
@@ -4539,8 +4542,7 @@ Uses @code{lp64s/

Re: [PATCH v2 1/4] LoongArch: improved target configuration interface

2023-08-31 Thread Yujie Yang
On Thu, Aug 31, 2023 at 11:14:13AM +0800, Yujie Yang wrote:
> On Wed, Aug 30, 2023 at 09:36:22PM +, Joseph Myers wrote:
> > On Wed, 30 Aug 2023, Yang Yujie wrote:
> > 
> > > +A suffix @code{[/ARCH][/OPTION]...]} may follow immediately after the ABI
> > > +identifier to customize the compiler options for building the given set 
> > > of
> > > +libraries.  @code{ARCH} denotes the architecture name recognized by the
> > > +@code{-march=ARCH} compiler option, which acts as a basic target ISA
> > > +configuration that can be adjusted using the subsequent @code{OPTION}
> > > +suffixes, where each @code{OPTION} is a compiler option itself.
> > 
> > Since ARCH and OPTION are not literal strings of program source code, you 
> > should actually be using @var{arch} and @var{option} for them (and @dots{} 
> > instead of ..., since the ... isn't literal source code either).
> > 
> > This patch series also adds a new configure option --with-strict-align-lib 
> > that needs documenting in the corresponding patch.
> > 
> > -- 
> > Joseph S. Myers
> > jos...@codesourcery.com
> 
> Thanks for the review.  Does the following fix look good?
> If so, I will include these in the patchset.

Found something else to fix, I will post them in the next version
of the patchset.



Re: [PATCH v2 1/4] LoongArch: improved target configuration interface

2023-08-31 Thread Yujie Yang
On Thu, Aug 31, 2023 at 05:56:26PM +, Joseph Myers wrote:
> On Thu, 31 Aug 2023, Yujie Yang wrote:
> 
> > -If none of such suffix is present, the configured value of
> > -@option{--with-multilib-default} can be used as a common default suffix
> > -for all library ABI variants.  Otherwise, the default build option
> > -@code{-march=abi-default} is applied when building the variants without
> > -a suffix.
> > +If no such suffix is present for a given multilib variant, the
> > +configured value of @code{--with-multilib-default} is appended as a default
> > +suffix.  If @code{--with-multilib-default} is not given, the default build
> > +option @code{-march=abi-default} is applied when building the variants
> > +without a suffix.
> 
> @option is appropriate for --with-multilib-default and other configure 
> options; it shouldn't be changed to @code.
> 

Thanks! Fixed in the v3 patchset.



Re: [PING][PATCH] LoongArch: initial ada support on linux

2023-08-31 Thread Yujie Yang
On Thu, Aug 31, 2023 at 03:09:52PM +0200, Marc Poulhiès wrote:
> 
> Yang Yujie  writes:
> 
> Hello Yujie,
> 
> > gcc/ChangeLog:
> >
> > * ada/Makefile.rtl: Add LoongArch support.
> > * ada/libgnarl/s-linux__loongarch.ads: New.
> > * ada/libgnat/system-linux-loongarch.ads: New.
> > * config/loongarch/loongarch.h: mark normalized options
> > passed from driver to gnat1 as explicit for multilib.
> > ---
> >  gcc/ada/Makefile.rtl   |  49 +++
> >  gcc/ada/libgnarl/s-linux__loongarch.ads| 134 +++
> >  gcc/ada/libgnat/system-linux-loongarch.ads | 145 +
> 
> The Ada part of the patch looks correct, thanks.
> 
> >  gcc/config/loongarch/loongarch.h   |   4 +-
> >  4 files changed, 330 insertions(+), 2 deletions(-)
> > diff --git a/gcc/config/loongarch/loongarch.h 
> > b/gcc/config/loongarch/loongarch.h
> > index f8167875646..9887a7ac630 100644
> > --- a/gcc/config/loongarch/loongarch.h
> > +++ b/gcc/config/loongarch/loongarch.h
> > @@ -83,9 +83,9 @@ along with GCC; see the file COPYING3.  If not see
> >  /* CC1_SPEC is the set of arguments to pass to the compiler proper.  */
> >
> >  #undef CC1_SPEC
> > -#define CC1_SPEC "\
> > +#define CC1_SPEC "%{,ada:-gnatea} %{m*} \
> >  %{G*} \
> > -%(subtarget_cc1_spec)"
> > +%(subtarget_cc1_spec) %{,ada:-gnatez}"
> >
> >  /* Preprocessor specs.  */
> 
> This is outside of ada/ (so I don't have a say on it), but I'm curious
> about why you need to use -gnatea/-gnatez here?
> 
> Thanks,
> Marc

Hi Marc,

Thank you for the review!

We added -gnatea and -gnatez to CC1_SPECS for correct multilib handling,
and I believe this is currently specific to LoongArch.

LoongArch relies on the GCC driver (via self_specs rules) to generate a
canonicalized tuple of parameters that identifies the current target (ISA/ABI)
configuration, including the "-mabi=" option that corresponds to the selected
multilib variant.  Even if "-mabi=" itself is not given explicitly to gcc, it
may be fed to the compiler propers with values other than the default ABI.

For GNAT on LoongArch, it is necessary that -mabi= generated by driver
self-specs gets stored in the .ali file, otherwise the linker might
hit the wrong multilib variant by assuming the default ABI.  Using
-gnatea/-gnatez can mark the driver-generated "-mabi=" as "explicit",
so it is sure to be found in "A"-records of the generated *.ali file.

Currently, gnatmake only marks user-specified options as explicit with
-gnatea and -gnatez, but not others [gcc/ada/make.adb].  So I think it's
necessary to have these marks around our driver-canonicalized %{m*} tuple
as well.

(Not sure if we should also mark non-multilib-related options other than
"-mabi=" as explicit, but it doesn't seem to do any harm.)

Sincerely,
Yujie