Re: [PATCH v1 2/6] LoongArch: improved target configuration interface
On Wed, Sep 06, 2023 at 07:43:26AM +0100, Richard Sandiford wrote:
> Yang Yujie writes:
> > @@ -5171,25 +5213,21 @@ case "${target}" in
> > # ${with_multilib_list} should not contain whitespaces,
> > # consecutive commas or slashes.
> > if echo "${with_multilib_list}" \
> > - | grep -E -e "[[:space:]]" -e '[,/][,/]' -e '[,/]$' -e '^[,/]'
> > > /dev/null; then
> > + | grep -E -e "[[:space:]]" -e '[,/][,/]' -e '[,/]$' -e '^[,/]'
> > > /dev/null 2>&1; then
> > echo "Invalid argument to --with-multilib-list." 1>&2
> > exit 1
> > fi
> >
> > - unset component idx elem_abi_base elem_abi_ext elem_tmp
> > + unset component elem_abi_base elem_abi_ext elem_tmp parse_state
> > all_abis
> > for elem in $(echo "${with_multilib_list}" | tr ',' ' '); do
> > - idx=0
> > - while true; do
> > - idx=$((idx + 1))
> > - component=$(echo "${elem}" | awk -F'/' '{print
> > $'"${idx}"'}')
> > -
> > - case ${idx} in
> > - 1)
> > - # Component 1: Base ABI type
> > + unset elem_abi_base elem_abi_ext
> > + parse_state="abi-base"
> > +
> > + for component in $(echo "${elem}" | tr '/' ' '); do
> > + if test x${parse_state} = x"abi-base"; then
> > + # Base ABI type
> > case ${component} in
> > - lp64d) elem_tmp="ABI_BASE_LP64D,";;
> > - lp64f) elem_tmp="ABI_BASE_LP64F,";;
> > - lp64s) elem_tmp="ABI_BASE_LP64S,";;
> > + lp64d | lp64f | lp64s)
> > elem_tmp="ABI_BASE_$(tr a-z A-Z <<< ${component}),";;
>
> "<<<" isn't portable shell. Could you try with:
>
> echo ${component} | tr ...
>
> instead?
>
> As it stands, this causes a bootstrap failure with non-bash shells
> such as dash, even on non-Loongson targets.
>
> (Part of me wishes that we'd just standardise on bash. But since that
> isn't the policy, I sometimes use dash to pick up my own lapses.)
>
> Thanks,
> Richard
Sorry, I missed it when cleaning up the patches. :(
Thanks for the review. Will fix this in a new patch.
Yujie
Re: [PATCH v1 2/6] LoongArch: improved target configuration interface
Yang Yujie writes:
> @@ -5171,25 +5213,21 @@ case "${target}" in
> # ${with_multilib_list} should not contain whitespaces,
> # consecutive commas or slashes.
> if echo "${with_multilib_list}" \
> - | grep -E -e "[[:space:]]" -e '[,/][,/]' -e '[,/]$' -e '^[,/]'
> > /dev/null; then
> + | grep -E -e "[[:space:]]" -e '[,/][,/]' -e '[,/]$' -e '^[,/]'
> > /dev/null 2>&1; then
> echo "Invalid argument to --with-multilib-list." 1>&2
> exit 1
> fi
>
> - unset component idx elem_abi_base elem_abi_ext elem_tmp
> + unset component elem_abi_base elem_abi_ext elem_tmp parse_state
> all_abis
> for elem in $(echo "${with_multilib_list}" | tr ',' ' '); do
> - idx=0
> - while true; do
> - idx=$((idx + 1))
> - component=$(echo "${elem}" | awk -F'/' '{print
> $'"${idx}"'}')
> -
> - case ${idx} in
> - 1)
> - # Component 1: Base ABI type
> + unset elem_abi_base elem_abi_ext
> + parse_state="abi-base"
> +
> + for component in $(echo "${elem}" | tr '/' ' '); do
> + if test x${parse_state} = x"abi-base"; then
> + # Base ABI type
> case ${component} in
> - lp64d) elem_tmp="ABI_BASE_LP64D,";;
> - lp64f) elem_tmp="ABI_BASE_LP64F,";;
> - lp64s) elem_tmp="ABI_BASE_LP64S,";;
> + lp64d | lp64f | lp64s)
> elem_tmp="ABI_BASE_$(tr a-z A-Z <<< ${component}),";;
"<<<" isn't portable shell. Could you try with:
echo ${component} | tr ...
instead?
As it stands, this causes a bootstrap failure with non-bash shells
such as dash, even on non-Loongson targets.
(Part of me wishes that we'd just standardise on bash. But since that
isn't the policy, I sometimes use dash to pick up my own lapses.)
Thanks,
Richard
Re: [PATCH v1 2/6] LoongArch: improved target configuration interface
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 2/6] LoongArch: improved target configuration interface
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
On Mon, 2023-08-14 at 16:57 +0800, Yujie Yang wrote: > 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? I'm OK with that, but we need to document it clearly in invoke.texi. -- Xi Ruoyao School of Aerospace Science and Technology, Xidian University
Re: [PATCH v1 2/6] LoongArch: improved target configuration interface
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
Re: [PATCH v1 2/6] LoongArch: improved target configuration interface
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
On Mon, 2023-08-14 at 13:58 +0800, Xi Ruoyao via Gcc-patches 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).
Perhaps just "--with-feature" or "--with-loongarch-feature", then we can
even fold -mstrict-align here, like "--with-feature=lbt,strict-align".
--
Xi Ruoyao
School of Aerospace Science and Technology, Xidian University
Re: [PATCH v1 2/6] LoongArch: improved target configuration interface
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
Re: [PATCH v1 2/6] LoongArch: improved target configuration interface
On Mon, 2023-08-14 at 11:57 +0800, Yang Yujie wrote: > loongarch64) > - tune_pattern="loongarch64|la464" > - tune_default="la464" > + tune_pattern="native|abi-default|loongarch64|la464" I think we can remove tune_pattern completely. There is no reason to limit --with-tune setting based on --with-arch setting. -- Xi Ruoyao School of Aerospace Science and Technology, Xidian University
Re: [PATCH v1 2/6] LoongArch: improved target configuration interface
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
[PATCH v1 2/6] LoongArch: improved target configuration interface
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}.
* Mark some LoongArch-specific "flags" with overlapping state-changing
semantics as "driver deferred" so that they could be processed in the
order they appear in the GCC driver. In this way, the final result can
be canonicalized into "parameters" (options with "=") for reliable use in
specs rules or by the compiler proper.
* Enforce the priority of configuration paths (for ={fpu,tune,simd}):
-m > -march-implied > --with- > --with-arch-implied.
* Allow the user to control the compiler options used when building
GCC libraries for each multilib variant via --with-multilib-list
and --with-multilib-default. This could become more useful when
we have 32-bit support later.
Example 1: the following configure option
--with-multilib-list=lp64d/la464/mno-strict-align/msimd=lsx,lp64s/mfpu=32
| || |
-mabi=ABI -march=ARCH a list of other options
(mandatory) (optional) (optional)
builds two sets of libraries:
lp64d ABI (the default ABI if no --with-abi is given,
built with "-march=la464 -mno-strict-align")
lp64s ABI (built with "-march=abi-default -mfpu=32")
Example 2: the following 3 configure options
--with-arch=loongarch64
--with-multilib-list=lp64d,lp64f,lp64s/la464
--with-multilib-default=fixed/mno-strict-align/mfpu=64
|| |
-march=ARCH a list of other options
(optional)(optional)
is equivalent to (in terms of building libraries):
--with-multilib-list=\
lp64d/loongarch64/mno-strict-align/mfpu=64,\
lp64f/loongarch64/mno-strict-align/mfpu=64,\
lp64s/la464
Note:
1. the GCC driver and compiler proper does not support
"-march=fixed". "fixed" that appear here acts as a placeholder for
"use whatever ARCH in --with-arch=ARCH" (or the default value
of --with-arch=ARCH if --with-arch is not explicitly configured).
2. if the ARCH part is omitted, "-march=abi-default"
is used for building all library variants, which
practically means enabling the minimal ISA features
that can support the given ABI.
gcc/ChangeLog:
* gcc/config.gcc: Add new configuration options --with-arch
and --with-multilib-default; slightly adjust the handling
of configure-time defaults.
* gcc/config/loongarch/genopts/loongarch-strings: Add keyword
"abi-default" as in "-march=abi-default".
* gcc/config/loongarch/loongarch-str.h: Likewise
* gcc/config/loongarch/genopts/loongarch.opt.in: Mark flags
as "driver deferred" so that a state machine could be applied.
* gcc/config/loongarch/loongarch.opt: Likewise.
* gcc/config/loongarch/loongarch-cpu.cc: refactor code for
the new internal representation of -march=native.
* gcc/config/loongarch/loongarch-cpu.h: Likewise.
* gcc/config/loongarch/loongarch-c.cc: Likewise.
* gcc/config/loongarch/loongarch-def.c: add SIMD attributes
for struct loongarch_target.
* gcc/config/loongarch/loongarch-def.h: Likewise.
* gcc/config/loongarch/loongarch-driver.cc: Implement a state
machine to canonicalize flags into parameters.
* gcc/config/loongarch/loongarch-driver.h: Likewise.
* gcc/config/loongarch/loongarch-opts.cc: Use the new internal
target representation as input as well as the output.
* gcc/config/loongarch/loongarch-opts.h: Likewise.
* gcc/config/loongarch/loongarch.cc: Likewise
* gcc/config/loongarch/t-linux: Support building GCC libraries
with customized compiler options using specs.
* gcc/doc/invoke.texi: document -m[no-]l[a]sx and -msimd=.
* gcc/doc/install.texi: document --with-multilib-default
and --with-multilib list.
---
gcc/config.gcc| 253 +
.../loongarch/genopts/loongarch-strings | 8 +-
gcc/config/loongarch/genopts/loongarch.opt.in | 62 +--
gcc/config/loongarch/la464.md | 32 +-
gcc/config/loongarch/loongarch-c.cc | 4 +-
gcc/config/loongarch/loongarch-cpu.cc | 260 -
gcc/config/loongarch/loongarch-cpu.h | 3 +-
gcc/config/loongarch/loongarch-def.c | 55 ++-
gcc/config/loongarch/loongarch-def.h | 57 +--
gcc/config/loongarch/loongarch-driver.cc | 205 +-
gcc/config/loongarch/loongarch-driver.h | 40 +-
gcc/config/loongarch/loongarch-opts.cc| 352
