[v2] driver: Fix multilib_os_dir and multiarch_dir for those target use TARGET_COMPUTE_MULTILIB
Checks
| Context |
Check |
Description |
| rivoscibot/toolchain-ci-rivos-apply-patch |
success
|
Patch applied
|
| rivoscibot/toolchain-ci-rivos-lint |
success
|
Lint passed
|
| rivoscibot/toolchain-ci-rivos-build--newlib-rv64gcv-lp64d-multilib |
success
|
Build passed
|
| rivoscibot/toolchain-ci-rivos-build--linux-rv64gcv-lp64d-multilib |
success
|
Build passed
|
| rivoscibot/toolchain-ci-rivos-build--linux-rv64gc_zba_zbb_zbc_zbs-lp64d-multilib |
success
|
Build passed
|
| rivoscibot/toolchain-ci-rivos-test |
success
|
Testing passed
|
Commit Message
This patch fixes the multilib_os_dir and multiarch_dir for those targets
that use TARGET_COMPUTE_MULTILIB, since the TARGET_COMPUTE_MULTILIB hook
only update/fix the multilib_dir but not the multilib_os_dir and multiarch_dir,
so the multilib_os_dir and multiarch_dir are not set correctly for those targets.
Use RISC-V linux target (riscv64-unknown-linux-gnu) as an example:
```
$ riscv64-unknown-linux-gnu-gcc -print-multi-lib
.;
lib32/ilp32;@march=rv32imac@mabi=ilp32
lib32/ilp32d;@march=rv32imafdc@mabi=ilp32d
lib64/lp64;@march=rv64imac@mabi=lp64
lib64/lp64d;@march=rv64imafdc@mabi=lp64d
```
If we use the exactly same -march and -mabi options to compile a source file,
the multilib_os_dir and multiarch_dir are set correctly:
```
$ riscv64-unknown-linux-gnu-gcc -print-multi-os-directory -march=rv64imafdc -mabi=lp64d
../lib64/lp64d
$ riscv64-unknown-linux-gnu-gcc -print-multi-directory -march=rv64imafdc -mabi=lp64d
lib64/lp64d
```
However if we use the -march=rv64imafdcv -mabi=lp64d option to compile a source
file, the multilib_os_dir and multiarch_dir are not set correctly:
```
$ riscv64-unknown-linux-gnu-gcc -print-multi-os-directory -march=rv64imafdc -mabi=lp64d
lib64/lp64d
$ riscv64-unknown-linux-gnu-gcc -print-multi-directory -march=rv64imafdc -mabi=lp64d
lib64/lp64d
```
That's because the TARGET_COMPUTE_MULTILIB hook only update/fix the multilib_dir
but not the multilib_os_dir, so the multilib_os_dir is blank and will use same
value as multilib_dir, but that is not correct.
So we introduce second chance to fix the multilib_os_dir if it's not set, we do
also try to fix the multiarch_dir, because it may also not set correctly if
multilib_os_dir is not set.
Changes since v1:
- Fix non-multilib build.
- Fix fix indentation.
gcc/ChangeLog:
* gcc.c (find_multilib_os_dir_by_multilib_dir): New.
(set_multilib_dir): Fix multilib_os_dir and multiarch_dir
if multilib_os_dir is not set.
---
gcc/gcc.cc | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 107 insertions(+), 1 deletion(-)
Comments
On 3/10/25 2:26 AM, Kito Cheng wrote:
> This patch fixes the multilib_os_dir and multiarch_dir for those targets
> that use TARGET_COMPUTE_MULTILIB, since the TARGET_COMPUTE_MULTILIB hook
> only update/fix the multilib_dir but not the multilib_os_dir and multiarch_dir,
> so the multilib_os_dir and multiarch_dir are not set correctly for those targets.
Thankfully only RISC-V defines TARGET_COMPUTE_MULTILIB. Though that may
be an argument we should look to avoid whatever magic we're doing in there.
>
> Use RISC-V linux target (riscv64-unknown-linux-gnu) as an example:
>
> ```
> $ riscv64-unknown-linux-gnu-gcc -print-multi-lib
> .;
> lib32/ilp32;@march=rv32imac@mabi=ilp32
> lib32/ilp32d;@march=rv32imafdc@mabi=ilp32d
> lib64/lp64;@march=rv64imac@mabi=lp64
> lib64/lp64d;@march=rv64imafdc@mabi=lp64d
> ```
>
> If we use the exactly same -march and -mabi options to compile a source file,
> the multilib_os_dir and multiarch_dir are set correctly:
>
> ```
> $ riscv64-unknown-linux-gnu-gcc -print-multi-os-directory -march=rv64imafdc -mabi=lp64d
> ../lib64/lp64d
> $ riscv64-unknown-linux-gnu-gcc -print-multi-directory -march=rv64imafdc -mabi=lp64d
> lib64/lp64d
> ```
>
> However if we use the -march=rv64imafdcv -mabi=lp64d option to compile a source
> file, the multilib_os_dir and multiarch_dir are not set correctly:
> ```
> $ riscv64-unknown-linux-gnu-gcc -print-multi-os-directory -march=rv64imafdc -mabi=lp64d
> lib64/lp64d
> $ riscv64-unknown-linux-gnu-gcc -print-multi-directory -march=rv64imafdc -mabi=lp64d
> lib64/lp64d
> ```
>
> That's because the TARGET_COMPUTE_MULTILIB hook only update/fix the multilib_dir
> but not the multilib_os_dir, so the multilib_os_dir is blank and will use same
> value as multilib_dir, but that is not correct.
>
> So we introduce second chance to fix the multilib_os_dir if it's not set, we do
> also try to fix the multiarch_dir, because it may also not set correctly if
> multilib_os_dir is not set.
>
> Changes since v1:
> - Fix non-multilib build.
> - Fix fix indentation.
>
> gcc/ChangeLog:
>
> * gcc.c (find_multilib_os_dir_by_multilib_dir): New.
> (set_multilib_dir): Fix multilib_os_dir and multiarch_dir
> if multilib_os_dir is not set.
Given the fact this code is shared and I don't have a good handle on its
behavior and how the change potentially affects other targets, I'm
inclined to ask for this to wait for gcc-16 development to open and
backport into gcc-15.2 after soak time on the trunk.
Jeff
On Sun, 16 Mar 2025 11:23:07 -0600, Jeff Law wrote:
>
>
> On 3/10/25 2:26 AM, Kito Cheng wrote:
> > This patch fixes the multilib_os_dir and multiarch_dir for those targets
> > that use TARGET_COMPUTE_MULTILIB, since the TARGET_COMPUTE_MULTILIB hook
> > only update/fix the multilib_dir but not the multilib_os_dir and multiarch_dir,
> > so the multilib_os_dir and multiarch_dir are not set correctly for those targets.
> Thankfully only RISC-V defines TARGET_COMPUTE_MULTILIB. Though that may
> be an argument we should look to avoid whatever magic we're doing in there.
>
>
> >
> > Use RISC-V linux target (riscv64-unknown-linux-gnu) as an example:
> >
> > ```
> > $ riscv64-unknown-linux-gnu-gcc -print-multi-lib
> > .;
> > lib32/ilp32;@march=rv32imac@mabi=ilp32
> > lib32/ilp32d;@march=rv32imafdc@mabi=ilp32d
> > lib64/lp64;@march=rv64imac@mabi=lp64
> > lib64/lp64d;@march=rv64imafdc@mabi=lp64d
> > ```
> >
> > If we use the exactly same -march and -mabi options to compile a source file,
> > the multilib_os_dir and multiarch_dir are set correctly:
> >
> > ```
> > $ riscv64-unknown-linux-gnu-gcc -print-multi-os-directory -march=rv64imafdc -mabi=lp64d
> > ../lib64/lp64d
> > $ riscv64-unknown-linux-gnu-gcc -print-multi-directory -march=rv64imafdc -mabi=lp64d
> > lib64/lp64d
> > ```
> >
> > However if we use the -march=rv64imafdcv -mabi=lp64d option to compile a source
> > file, the multilib_os_dir and multiarch_dir are not set correctly:
> > ```
> > $ riscv64-unknown-linux-gnu-gcc -print-multi-os-directory -march=rv64imafdc -mabi=lp64d
> > lib64/lp64d
> > $ riscv64-unknown-linux-gnu-gcc -print-multi-directory -march=rv64imafdc -mabi=lp64d
> > lib64/lp64d
> > ```
> >
> > That's because the TARGET_COMPUTE_MULTILIB hook only update/fix the multilib_dir
> > but not the multilib_os_dir, so the multilib_os_dir is blank and will use same
> > value as multilib_dir, but that is not correct.
> >
> > So we introduce second chance to fix the multilib_os_dir if it's not set, we do
> > also try to fix the multiarch_dir, because it may also not set correctly if
> > multilib_os_dir is not set.
> >
> > Changes since v1:
> > - Fix non-multilib build.
> > - Fix fix indentation.
> >
> > gcc/ChangeLog:
> >
> > * gcc.c (find_multilib_os_dir_by_multilib_dir): New.
> > (set_multilib_dir): Fix multilib_os_dir and multiarch_dir
> > if multilib_os_dir is not set.
> Given the fact this code is shared and I don't have a good handle on its
> behavior and how the change potentially affects other targets, I'm
> inclined to ask for this to wait for gcc-16 development to open and
> backport into gcc-15.2 after soak time on the trunk.
>
> Jeff
Hi,I think this patch is essential. Can we proceed to push it to the trunk now?
Best regards,
Jin Ma
Hi Jin:
Thanks for heads up:)
Hi Jeff:
I've rebased that on the trunk and everything seems right, do you think
it's OK for the trunk?
On Mon, May 19, 2025 at 2:35 PM Jin Ma <jinma@linux.alibaba.com> wrote:
> On Sun, 16 Mar 2025 11:23:07 -0600, Jeff Law wrote:
> >
> >
> > On 3/10/25 2:26 AM, Kito Cheng wrote:
> > > This patch fixes the multilib_os_dir and multiarch_dir for those
> targets
> > > that use TARGET_COMPUTE_MULTILIB, since the TARGET_COMPUTE_MULTILIB
> hook
> > > only update/fix the multilib_dir but not the multilib_os_dir and
> multiarch_dir,
> > > so the multilib_os_dir and multiarch_dir are not set correctly for
> those targets.
> > Thankfully only RISC-V defines TARGET_COMPUTE_MULTILIB. Though that may
> > be an argument we should look to avoid whatever magic we're doing in
> there.
> >
> >
> > >
> > > Use RISC-V linux target (riscv64-unknown-linux-gnu) as an example:
> > >
> > > ```
> > > $ riscv64-unknown-linux-gnu-gcc -print-multi-lib
> > > .;
> > > lib32/ilp32;@march=rv32imac@mabi=ilp32
> > > lib32/ilp32d;@march=rv32imafdc@mabi=ilp32d
> > > lib64/lp64;@march=rv64imac@mabi=lp64
> > > lib64/lp64d;@march=rv64imafdc@mabi=lp64d
> > > ```
> > >
> > > If we use the exactly same -march and -mabi options to compile a
> source file,
> > > the multilib_os_dir and multiarch_dir are set correctly:
> > >
> > > ```
> > > $ riscv64-unknown-linux-gnu-gcc -print-multi-os-directory
> -march=rv64imafdc -mabi=lp64d
> > > ../lib64/lp64d
> > > $ riscv64-unknown-linux-gnu-gcc -print-multi-directory
> -march=rv64imafdc -mabi=lp64d
> > > lib64/lp64d
> > > ```
> > >
> > > However if we use the -march=rv64imafdcv -mabi=lp64d option to compile
> a source
> > > file, the multilib_os_dir and multiarch_dir are not set correctly:
> > > ```
> > > $ riscv64-unknown-linux-gnu-gcc -print-multi-os-directory
> -march=rv64imafdc -mabi=lp64d
> > > lib64/lp64d
> > > $ riscv64-unknown-linux-gnu-gcc -print-multi-directory
> -march=rv64imafdc -mabi=lp64d
> > > lib64/lp64d
> > > ```
> > >
> > > That's because the TARGET_COMPUTE_MULTILIB hook only update/fix the
> multilib_dir
> > > but not the multilib_os_dir, so the multilib_os_dir is blank and will
> use same
> > > value as multilib_dir, but that is not correct.
> > >
> > > So we introduce second chance to fix the multilib_os_dir if it's not
> set, we do
> > > also try to fix the multiarch_dir, because it may also not set
> correctly if
> > > multilib_os_dir is not set.
> > >
> > > Changes since v1:
> > > - Fix non-multilib build.
> > > - Fix fix indentation.
> > >
> > > gcc/ChangeLog:
> > >
> > > * gcc.c (find_multilib_os_dir_by_multilib_dir): New.
> > > (set_multilib_dir): Fix multilib_os_dir and multiarch_dir
> > > if multilib_os_dir is not set.
> > Given the fact this code is shared and I don't have a good handle on its
> > behavior and how the change potentially affects other targets, I'm
> > inclined to ask for this to wait for gcc-16 development to open and
> > backport into gcc-15.2 after soak time on the trunk.
> >
> > Jeff
>
> Hi,I think this patch is essential. Can we proceed to push it to the trunk
> now?
>
> Best regards,
> Jin Ma
On 5/19/25 12:48 AM, Kito Cheng wrote:
> Hi Jin:
>
> Thanks for heads up:)
>
> Hi Jeff:
>
> I've rebased that on the trunk and everything seems right, do you think
> it's OK for the trunk?
Yea, let's get it on the trunk and get it some soak time. We can then
look at backporting it to gcc-15's release branch.
jeff
Pushed to trunk :)
On Wed, May 21, 2025 at 2:35 AM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 5/19/25 12:48 AM, Kito Cheng wrote:
> > Hi Jin:
> >
> > Thanks for heads up:)
> >
> > Hi Jeff:
> >
> > I've rebased that on the trunk and everything seems right, do you think
> > it's OK for the trunk?
> Yea, let's get it on the trunk and get it some soak time. We can then
> look at backporting it to gcc-15's release branch.
>
> jeff
>
@@ -9736,6 +9736,103 @@ default_arg (const char *p, int len)
return 0;
}
+/* Use multilib_dir as key to find corresponding multilib_os_dir and
+ multiarch_dir. */
+
+static void
+find_multilib_os_dir_by_multilib_dir (const char *multilib_dir,
+ const char **p_multilib_os_dir,
+ const char **p_multiarch_dir)
+{
+ const char *p = multilib_select;
+ unsigned int this_path_len;
+ const char *this_path;
+ int ok = 0;
+
+ while (*p != '\0')
+ {
+ /* Ignore newlines. */
+ if (*p == '\n')
+ {
+ ++p;
+ continue;
+ }
+
+ /* Get the initial path. */
+ this_path = p;
+ while (*p != ' ')
+ {
+ if (*p == '\0')
+ {
+ fatal_error (input_location, "multilib select %qs %qs is invalid",
+ multilib_select, multilib_reuse);
+ }
+ ++p;
+ }
+ this_path_len = p - this_path;
+
+ ok = 0;
+
+ /* Skip any arguments, we don't care at this stage. */
+ while (*++p != ';');
+
+ if (this_path_len != 1
+ || this_path[0] != '.')
+ {
+ char *new_multilib_dir = XNEWVEC (char, this_path_len + 1);
+ char *q;
+
+ strncpy (new_multilib_dir, this_path, this_path_len);
+ new_multilib_dir[this_path_len] = '\0';
+ q = strchr (new_multilib_dir, ':');
+ if (q != NULL)
+ *q = '\0';
+
+ if (strcmp (new_multilib_dir, multilib_dir) == 0)
+ ok = 1;
+ }
+
+ /* Found matched multilib_dir, update multilib_os_dir and
+ multiarch_dir. */
+ if (ok)
+ {
+ const char *q = this_path, *end = this_path + this_path_len;
+
+ while (q < end && *q != ':')
+ q++;
+ if (q < end)
+ {
+ const char *q2 = q + 1, *ml_end = end;
+ char *new_multilib_os_dir;
+
+ while (q2 < end && *q2 != ':')
+ q2++;
+ if (*q2 == ':')
+ ml_end = q2;
+ if (ml_end - q == 1)
+ *p_multilib_os_dir = xstrdup (".");
+ else
+ {
+ new_multilib_os_dir = XNEWVEC (char, ml_end - q);
+ memcpy (new_multilib_os_dir, q + 1, ml_end - q - 1);
+ new_multilib_os_dir[ml_end - q - 1] = '\0';
+ *p_multilib_os_dir = new_multilib_os_dir;
+ }
+
+ if (q2 < end && *q2 == ':')
+ {
+ char *new_multiarch_dir = XNEWVEC (char, end - q2);
+ memcpy (new_multiarch_dir, q2 + 1, end - q2 - 1);
+ new_multiarch_dir[end - q2 - 1] = '\0';
+ *p_multiarch_dir = new_multiarch_dir;
+ }
+ break;
+ }
+ }
+ ++p;
+ }
+}
+
/* Work out the subdirectory to use based on the options. The format of
multilib_select is a list of elements. Each element is a subdirectory
name followed by a list of options followed by a semicolon. The format
@@ -10014,7 +10111,16 @@ set_multilib_dir (void)
multilib_os_dir = NULL;
}
else if (multilib_dir != NULL && multilib_os_dir == NULL)
- multilib_os_dir = multilib_dir;
+ {
+ /* Give second chance to search matched multilib_os_dir again by matching
+ the multilib_dir since some target may use TARGET_COMPUTE_MULTILIB
+ hook rather than the builtin way. */
+ find_multilib_os_dir_by_multilib_dir (multilib_dir, &multilib_os_dir,
+ &multiarch_dir);
+
+ if (multilib_os_dir == NULL)
+ multilib_os_dir = multilib_dir;
+ }
}
/* Print out the multiple library subdirectory selection