public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [RFC] decay vect tests from run to link for pr95401
@ 2021-03-11  7:38 Alexandre Oliva
  2021-03-11  9:48 ` Richard Biener
  0 siblings, 1 reply; 12+ messages in thread
From: Alexandre Oliva @ 2021-03-11  7:38 UTC (permalink / raw)
  To: gcc-patches


When vect.exp finds our configuration disables altivec by default, it
disables the execution of vectorization tests, assuming the test
hardware doesn't support it.

Tests become just compile tests, but compile tests won't work
correctly when additional sources are named, e.g. pr95401.cc, because
GCC refuses to compile multiple files into the same asm output.

With this patch, the default for when execution is not possible
becomes link.


This was regstrapped on x86_64-linux-gnu and ppc64-linux-gnu, and tested
with a cross to a ppc64-vxworks7r2 with altivec disabled by default.  I
found fixing the handling of additional sources to e.g. compile each one
separately, or perhaps just discard or reject additional sources for
compile tests, to be a little too involved.

So I'm leaning towards this proposed change, just extended to other
platforms that also decay from run to compile rather than link, and thus
run into this problem in g++.dg/vect/pr95401.cc.  Would this be
acceptable?


for  gcc/testsuite/ChangeLog

	* lib/target-supports.exp (check_vect_support_and_set_flags):
	Decay to link rather than compile.
---
 gcc/testsuite/lib/target-supports.exp |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 52d3d036d3c5c..23d532fb2b963 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -9656,7 +9656,7 @@ proc check_vect_support_and_set_flags { } {
                 # Specify a cpu that supports VMX for compile-only tests.
                 lappend DEFAULT_VECTCFLAGS "-mcpu=970"
             }
-            set dg-do-what-default compile
+            set dg-do-what-default link
         }
     } elseif { [istarget i?86-*-*] || [istarget x86_64-*-*] } {
         lappend DEFAULT_VECTCFLAGS "-msse2"

-- 
Alexandre Oliva, happy hacker  https://FSFLA.org/blogs/lxo/
   Free Software Activist         GNU Toolchain Engineer
        Vim, Vi, Voltei pro Emacs -- GNUlius Caesar

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] decay vect tests from run to link for pr95401
  2021-03-11  7:38 [RFC] decay vect tests from run to link for pr95401 Alexandre Oliva
@ 2021-03-11  9:48 ` Richard Biener
  2021-03-11 12:17   ` Richard Sandiford
                     ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Richard Biener @ 2021-03-11  9:48 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: GCC Patches

On Thu, Mar 11, 2021 at 9:03 AM Alexandre Oliva <oliva@adacore.com> wrote:
>
>
> When vect.exp finds our configuration disables altivec by default, it
> disables the execution of vectorization tests, assuming the test
> hardware doesn't support it.
>
> Tests become just compile tests, but compile tests won't work
> correctly when additional sources are named, e.g. pr95401.cc, because
> GCC refuses to compile multiple files into the same asm output.
>
> With this patch, the default for when execution is not possible
> becomes link.
>
>
> This was regstrapped on x86_64-linux-gnu and ppc64-linux-gnu, and tested
> with a cross to a ppc64-vxworks7r2 with altivec disabled by default.  I
> found fixing the handling of additional sources to e.g. compile each one
> separately, or perhaps just discard or reject additional sources for
> compile tests, to be a little too involved.
>
> So I'm leaning towards this proposed change, just extended to other
> platforms that also decay from run to compile rather than link, and thus
> run into this problem in g++.dg/vect/pr95401.cc.  Would this be
> acceptable?

I think that's OK.  It's probably difficult to make the test UNSUPPORTED
when dg-additional-sources is discovered with a dg-do compile test?

Richard.

>
> for  gcc/testsuite/ChangeLog
>
>         * lib/target-supports.exp (check_vect_support_and_set_flags):
>         Decay to link rather than compile.
> ---
>  gcc/testsuite/lib/target-supports.exp |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
> index 52d3d036d3c5c..23d532fb2b963 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -9656,7 +9656,7 @@ proc check_vect_support_and_set_flags { } {
>                  # Specify a cpu that supports VMX for compile-only tests.
>                  lappend DEFAULT_VECTCFLAGS "-mcpu=970"
>              }
> -            set dg-do-what-default compile
> +            set dg-do-what-default link
>          }
>      } elseif { [istarget i?86-*-*] || [istarget x86_64-*-*] } {
>          lappend DEFAULT_VECTCFLAGS "-msse2"
>
> --
> Alexandre Oliva, happy hacker  https://FSFLA.org/blogs/lxo/
>    Free Software Activist         GNU Toolchain Engineer
>         Vim, Vi, Voltei pro Emacs -- GNUlius Caesar

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] decay vect tests from run to link for pr95401
  2021-03-11  9:48 ` Richard Biener
@ 2021-03-11 12:17   ` Richard Sandiford
  2021-03-11 14:47   ` Alexandre Oliva
  2024-04-22 10:05   ` [PATCH] " Alexandre Oliva
  2 siblings, 0 replies; 12+ messages in thread
From: Richard Sandiford @ 2021-03-11 12:17 UTC (permalink / raw)
  To: Richard Biener via Gcc-patches; +Cc: Alexandre Oliva, Richard Biener

Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> On Thu, Mar 11, 2021 at 9:03 AM Alexandre Oliva <oliva@adacore.com> wrote:
>>
>>
>> When vect.exp finds our configuration disables altivec by default, it
>> disables the execution of vectorization tests, assuming the test
>> hardware doesn't support it.
>>
>> Tests become just compile tests, but compile tests won't work
>> correctly when additional sources are named, e.g. pr95401.cc, because
>> GCC refuses to compile multiple files into the same asm output.
>>
>> With this patch, the default for when execution is not possible
>> becomes link.
>>
>>
>> This was regstrapped on x86_64-linux-gnu and ppc64-linux-gnu, and tested
>> with a cross to a ppc64-vxworks7r2 with altivec disabled by default.  I
>> found fixing the handling of additional sources to e.g. compile each one
>> separately, or perhaps just discard or reject additional sources for
>> compile tests, to be a little too involved.
>>
>> So I'm leaning towards this proposed change, just extended to other
>> platforms that also decay from run to compile rather than link, and thus
>> run into this problem in g++.dg/vect/pr95401.cc.  Would this be
>> acceptable?
>
> I think that's OK.

+1 FWIW.  It seems like an improvement anyway, since it makes it
harder to forget an explicit dg-do compile in cases where it's needed.

Richard



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] decay vect tests from run to link for pr95401
  2021-03-11  9:48 ` Richard Biener
  2021-03-11 12:17   ` Richard Sandiford
@ 2021-03-11 14:47   ` Alexandre Oliva
  2021-03-12  7:42     ` Richard Biener
  2024-04-22 10:05   ` [PATCH] " Alexandre Oliva
  2 siblings, 1 reply; 12+ messages in thread
From: Alexandre Oliva @ 2021-03-11 14:47 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

On Mar 11, 2021, Richard Biener <richard.guenther@gmail.com> wrote:

> I think that's OK.

Cool, here's the patch I'm nearly done regstrapping on x86_64-linux-gnu
and x-ppc64-vx7r2.  Ok to install?

> It's probably difficult to make the test UNSUPPORTED
> when dg-additional-sources is discovered with a dg-do compile test?

Well, first of all, I really don't like the idea of skipping a test if
we can still get some useful information out of it.

That said, I suppose we could test for additional_sources_used in
${langdriver}_target_compile proces in gcc.exp, g++.exp et al, between
their calling dg-additional-files-options and target_compile, to
conditionally skip the latter, or pass $type to the former in all
$langdriver.exp, so that the extra files can be flagged and/or discarded
in unsupported modes.  I believe such changes would also require
adjustments to library test infrastructures.


decay vect tests from run to link for pr95401

When vect.exp finds our configuration disables altivec by default, it
disables the execution of vectorization tests, assuming the test
hardware doesn't support it.

Tests become just compile tests, but compile tests won't work
correctly when additional sources are named, e.g. pr95401.cc, because
GCC refuses to compile multiple files into the same asm output.

With this patch, the default for when execution is not possible
becomes link.


for  gcc/testsuite/ChangeLog

	* lib/target-supports.exp (check_vect_support_and_set_flags):
	Decay to link rather than compile.
---
 gcc/testsuite/lib/target-supports.exp |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 52d3d036d3c5c..f5b9b0578de37 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -9632,7 +9632,7 @@ proc check_vect_support_and_set_flags { } {
         if [check_750cl_hw_available] {
             set dg-do-what-default run
         } else {
-            set dg-do-what-default compile
+            set dg-do-what-default link
         }
     } elseif [istarget powerpc*-*-*] {
         # Skip targets not supporting -maltivec.
@@ -9656,14 +9656,14 @@ proc check_vect_support_and_set_flags { } {
                 # Specify a cpu that supports VMX for compile-only tests.
                 lappend DEFAULT_VECTCFLAGS "-mcpu=970"
             }
-            set dg-do-what-default compile
+            set dg-do-what-default link
         }
     } elseif { [istarget i?86-*-*] || [istarget x86_64-*-*] } {
         lappend DEFAULT_VECTCFLAGS "-msse2"
         if { [check_effective_target_sse2_runtime] } {
             set dg-do-what-default run
         } else {
-            set dg-do-what-default compile
+            set dg-do-what-default link
         }
     } elseif { [istarget mips*-*-*]
 	       && [check_effective_target_nomips16] } {
@@ -9682,7 +9682,7 @@ proc check_vect_support_and_set_flags { } {
         if [check_effective_target_ultrasparc_hw] {
             set dg-do-what-default run
         } else {
-            set dg-do-what-default compile
+            set dg-do-what-default link
         }
     } elseif [istarget alpha*-*-*] {
         # Alpha's vectorization capabilities are extremely limited.
@@ -9695,7 +9695,7 @@ proc check_vect_support_and_set_flags { } {
         if [check_alpha_max_hw_available] {
             set dg-do-what-default run
         } else {
-            set dg-do-what-default compile
+            set dg-do-what-default link
         }
     } elseif [istarget ia64-*-*] {
         set dg-do-what-default run
@@ -9708,7 +9708,7 @@ proc check_vect_support_and_set_flags { } {
         if [is-effective-target arm_neon_hw] {
             set dg-do-what-default run
         } else {
-            set dg-do-what-default compile
+            set dg-do-what-default link
         }
     } elseif [istarget "aarch64*-*-*"] {
         set dg-do-what-default run
@@ -9729,7 +9729,7 @@ proc check_vect_support_and_set_flags { } {
             set dg-do-what-default run
         } else {
 	    lappend DEFAULT_VECTCFLAGS "-march=z14" "-mzarch"
-            set dg-do-what-default compile
+            set dg-do-what-default link
         }
     } elseif [istarget amdgcn-*-*] {
         set dg-do-what-default run


-- 
Alexandre Oliva, happy hacker  https://FSFLA.org/blogs/lxo/
   Free Software Activist         GNU Toolchain Engineer
        Vim, Vi, Voltei pro Emacs -- GNUlius Caesar

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] decay vect tests from run to link for pr95401
  2021-03-11 14:47   ` Alexandre Oliva
@ 2021-03-12  7:42     ` Richard Biener
  0 siblings, 0 replies; 12+ messages in thread
From: Richard Biener @ 2021-03-12  7:42 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: GCC Patches

On Thu, Mar 11, 2021 at 3:47 PM Alexandre Oliva <oliva@adacore.com> wrote:
>
> On Mar 11, 2021, Richard Biener <richard.guenther@gmail.com> wrote:
>
> > I think that's OK.
>
> Cool, here's the patch I'm nearly done regstrapping on x86_64-linux-gnu
> and x-ppc64-vx7r2.  Ok to install?

OK.

Richard.

> > It's probably difficult to make the test UNSUPPORTED
> > when dg-additional-sources is discovered with a dg-do compile test?
>
> Well, first of all, I really don't like the idea of skipping a test if
> we can still get some useful information out of it.
>
> That said, I suppose we could test for additional_sources_used in
> ${langdriver}_target_compile proces in gcc.exp, g++.exp et al, between
> their calling dg-additional-files-options and target_compile, to
> conditionally skip the latter, or pass $type to the former in all
> $langdriver.exp, so that the extra files can be flagged and/or discarded
> in unsupported modes.  I believe such changes would also require
> adjustments to library test infrastructures.
>
>
> decay vect tests from run to link for pr95401
>
> When vect.exp finds our configuration disables altivec by default, it
> disables the execution of vectorization tests, assuming the test
> hardware doesn't support it.
>
> Tests become just compile tests, but compile tests won't work
> correctly when additional sources are named, e.g. pr95401.cc, because
> GCC refuses to compile multiple files into the same asm output.
>
> With this patch, the default for when execution is not possible
> becomes link.
>
>
> for  gcc/testsuite/ChangeLog
>
>         * lib/target-supports.exp (check_vect_support_and_set_flags):
>         Decay to link rather than compile.
> ---
>  gcc/testsuite/lib/target-supports.exp |   14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
> index 52d3d036d3c5c..f5b9b0578de37 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -9632,7 +9632,7 @@ proc check_vect_support_and_set_flags { } {
>          if [check_750cl_hw_available] {
>              set dg-do-what-default run
>          } else {
> -            set dg-do-what-default compile
> +            set dg-do-what-default link
>          }
>      } elseif [istarget powerpc*-*-*] {
>          # Skip targets not supporting -maltivec.
> @@ -9656,14 +9656,14 @@ proc check_vect_support_and_set_flags { } {
>                  # Specify a cpu that supports VMX for compile-only tests.
>                  lappend DEFAULT_VECTCFLAGS "-mcpu=970"
>              }
> -            set dg-do-what-default compile
> +            set dg-do-what-default link
>          }
>      } elseif { [istarget i?86-*-*] || [istarget x86_64-*-*] } {
>          lappend DEFAULT_VECTCFLAGS "-msse2"
>          if { [check_effective_target_sse2_runtime] } {
>              set dg-do-what-default run
>          } else {
> -            set dg-do-what-default compile
> +            set dg-do-what-default link
>          }
>      } elseif { [istarget mips*-*-*]
>                && [check_effective_target_nomips16] } {
> @@ -9682,7 +9682,7 @@ proc check_vect_support_and_set_flags { } {
>          if [check_effective_target_ultrasparc_hw] {
>              set dg-do-what-default run
>          } else {
> -            set dg-do-what-default compile
> +            set dg-do-what-default link
>          }
>      } elseif [istarget alpha*-*-*] {
>          # Alpha's vectorization capabilities are extremely limited.
> @@ -9695,7 +9695,7 @@ proc check_vect_support_and_set_flags { } {
>          if [check_alpha_max_hw_available] {
>              set dg-do-what-default run
>          } else {
> -            set dg-do-what-default compile
> +            set dg-do-what-default link
>          }
>      } elseif [istarget ia64-*-*] {
>          set dg-do-what-default run
> @@ -9708,7 +9708,7 @@ proc check_vect_support_and_set_flags { } {
>          if [is-effective-target arm_neon_hw] {
>              set dg-do-what-default run
>          } else {
> -            set dg-do-what-default compile
> +            set dg-do-what-default link
>          }
>      } elseif [istarget "aarch64*-*-*"] {
>          set dg-do-what-default run
> @@ -9729,7 +9729,7 @@ proc check_vect_support_and_set_flags { } {
>              set dg-do-what-default run
>          } else {
>             lappend DEFAULT_VECTCFLAGS "-march=z14" "-mzarch"
> -            set dg-do-what-default compile
> +            set dg-do-what-default link
>          }
>      } elseif [istarget amdgcn-*-*] {
>          set dg-do-what-default run
>
>
> --
> Alexandre Oliva, happy hacker  https://FSFLA.org/blogs/lxo/
>    Free Software Activist         GNU Toolchain Engineer
>         Vim, Vi, Voltei pro Emacs -- GNUlius Caesar

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] decay vect tests from run to link for pr95401
  2021-03-11  9:48 ` Richard Biener
  2021-03-11 12:17   ` Richard Sandiford
  2021-03-11 14:47   ` Alexandre Oliva
@ 2024-04-22 10:05   ` Alexandre Oliva
  2024-04-22 19:10     ` Richard Biener
  2 siblings, 1 reply; 12+ messages in thread
From: Alexandre Oliva @ 2024-04-22 10:05 UTC (permalink / raw)
  To: Richard Biener
  Cc: GCC Patches, Rainer Orth, Mike Stump, David Edelsohn,
	Segher Boessenkool, Kewen Lin

Ping?-ish for the full version of the RFC posted at
https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566588.html

On Mar 11, 2021, Richard Biener <richard.guenther@gmail.com> wrote:

> On Thu, Mar 11, 2021 at 9:03 AM Alexandre Oliva <oliva@adacore.com> wrote:

>> So I'm leaning towards this proposed change, just extended to other
>> platforms that also decay from run to compile rather than link, and thus
>> run into this problem in g++.dg/vect/pr95401.cc.  Would this be
>> acceptable?

> I think that's OK.  It's probably difficult to make the test UNSUPPORTED
> when dg-additional-sources is discovered with a dg-do compile test?

Thanks, here's a completed version.


When vect.exp finds our configuration disables altivec by default, it
disables the execution of vectorization tests, assuming the test
hardware doesn't support it.

Tests become just compile tests, but compile tests won't work
correctly when additional sources are named, e.g. pr95401.cc, because
GCC refuses to compile multiple files into the same asm output.

With this patch, the default for when execution is not possible
becomes link.

Regstrapped on x86_64-linux-gnu and ppc64el-linux-gnu.  Also tested with
gcc-13 on ppc64-vx7r2 and ppc-vx7r2.  Ok to install?


for  gcc/testsuite/ChangeLog

	* lib/target-supports.exp (check_vect_support_and_set_flags):
	Decay to link rather than compile.
---
 gcc/testsuite/lib/target-supports.exp |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 3a5713d98691f..54a55585371b0 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -11625,7 +11625,7 @@ proc check_vect_support_and_set_flags { } {
         if [check_750cl_hw_available] {
             set dg-do-what-default run
         } else {
-            set dg-do-what-default compile
+            set dg-do-what-default link
         }
     } elseif [istarget powerpc*-*-*] {
         # Skip targets not supporting -maltivec.
@@ -11655,14 +11655,14 @@ proc check_vect_support_and_set_flags { } {
                 # some other cpu type specified above.
 		set DEFAULT_VECTCFLAGS [linsert $DEFAULT_VECTCFLAGS 0 "-mcpu=970"]
             }
-            set dg-do-what-default compile
+            set dg-do-what-default link
         }
     } elseif { [istarget i?86-*-*] || [istarget x86_64-*-*] } {
         lappend DEFAULT_VECTCFLAGS "-msse2"
         if { [check_effective_target_sse2_runtime] } {
             set dg-do-what-default run
         } else {
-            set dg-do-what-default compile
+            set dg-do-what-default link
         }
     } elseif { [istarget mips*-*-*]
 	       && [check_effective_target_nomips16] } {
@@ -11681,7 +11681,7 @@ proc check_vect_support_and_set_flags { } {
         if [check_effective_target_ultrasparc_hw] {
             set dg-do-what-default run
         } else {
-            set dg-do-what-default compile
+            set dg-do-what-default link
         }
     } elseif [istarget alpha*-*-*] {
         # Alpha's vectorization capabilities are extremely limited.
@@ -11694,7 +11694,7 @@ proc check_vect_support_and_set_flags { } {
         if [check_alpha_max_hw_available] {
             set dg-do-what-default run
         } else {
-            set dg-do-what-default compile
+            set dg-do-what-default link
         }
     } elseif [istarget ia64-*-*] {
         set dg-do-what-default run
@@ -11707,7 +11707,7 @@ proc check_vect_support_and_set_flags { } {
         if [is-effective-target arm_neon_hw] {
             set dg-do-what-default run
         } else {
-            set dg-do-what-default compile
+            set dg-do-what-default link
         }
     } elseif [istarget aarch64*-*-*] {
         set dg-do-what-default run
@@ -11731,7 +11731,7 @@ proc check_vect_support_and_set_flags { } {
             set dg-do-what-default run
         } else {
 	    lappend DEFAULT_VECTCFLAGS "-march=z14" "-mzarch"
-            set dg-do-what-default compile
+            set dg-do-what-default link
         }
     } elseif [istarget amdgcn-*-*] {
         set dg-do-what-default run
@@ -11742,7 +11742,7 @@ proc check_vect_support_and_set_flags { } {
 	    foreach item [add_options_for_riscv_v ""] {
 		lappend DEFAULT_VECTCFLAGS $item
 	    }
-	    set dg-do-what-default compile
+	    set dg-do-what-default link
 	}
     } elseif [istarget loongarch*-*-*] {
       # Set the default vectorization option to "-mlsx" due to the problem
@@ -11751,7 +11751,7 @@ proc check_vect_support_and_set_flags { } {
       if [check_effective_target_loongarch_sx_hw] {
 	  set dg-do-what-default run
       } else {
-	  set dg-do-what-default compile
+	  set dg-do-what-default link
       }
     } else {
         return 0


-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] decay vect tests from run to link for pr95401
  2024-04-22 10:05   ` [PATCH] " Alexandre Oliva
@ 2024-04-22 19:10     ` Richard Biener
  2024-04-29 23:30       ` Alexandre Oliva
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Biener @ 2024-04-22 19:10 UTC (permalink / raw)
  To: Alexandre Oliva
  Cc: GCC Patches, Rainer Orth, Mike Stump, David Edelsohn,
	Segher Boessenkool, Kewen Lin

On Mon, Apr 22, 2024 at 12:05 PM Alexandre Oliva <oliva@adacore.com> wrote:
>
> Ping?-ish for the full version of the RFC posted at
> https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566588.html
>
> On Mar 11, 2021, Richard Biener <richard.guenther@gmail.com> wrote:
>
> > On Thu, Mar 11, 2021 at 9:03 AM Alexandre Oliva <oliva@adacore.com> wrote:
>
> >> So I'm leaning towards this proposed change, just extended to other
> >> platforms that also decay from run to compile rather than link, and thus
> >> run into this problem in g++.dg/vect/pr95401.cc.  Would this be
> >> acceptable?
>
> > I think that's OK.  It's probably difficult to make the test UNSUPPORTED
> > when dg-additional-sources is discovered with a dg-do compile test?
>
> Thanks, here's a completed version.
>
>
> When vect.exp finds our configuration disables altivec by default, it
> disables the execution of vectorization tests, assuming the test
> hardware doesn't support it.
>
> Tests become just compile tests, but compile tests won't work
> correctly when additional sources are named, e.g. pr95401.cc, because
> GCC refuses to compile multiple files into the same asm output.
>
> With this patch, the default for when execution is not possible
> becomes link.
>
> Regstrapped on x86_64-linux-gnu and ppc64el-linux-gnu.  Also tested with
> gcc-13 on ppc64-vx7r2 and ppc-vx7r2.  Ok to install?

That makes sense.  OK

Thanks,
Richard.

>
> for  gcc/testsuite/ChangeLog
>
>         * lib/target-supports.exp (check_vect_support_and_set_flags):
>         Decay to link rather than compile.
> ---
>  gcc/testsuite/lib/target-supports.exp |   18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
> index 3a5713d98691f..54a55585371b0 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -11625,7 +11625,7 @@ proc check_vect_support_and_set_flags { } {
>          if [check_750cl_hw_available] {
>              set dg-do-what-default run
>          } else {
> -            set dg-do-what-default compile
> +            set dg-do-what-default link
>          }
>      } elseif [istarget powerpc*-*-*] {
>          # Skip targets not supporting -maltivec.
> @@ -11655,14 +11655,14 @@ proc check_vect_support_and_set_flags { } {
>                  # some other cpu type specified above.
>                 set DEFAULT_VECTCFLAGS [linsert $DEFAULT_VECTCFLAGS 0 "-mcpu=970"]
>              }
> -            set dg-do-what-default compile
> +            set dg-do-what-default link
>          }
>      } elseif { [istarget i?86-*-*] || [istarget x86_64-*-*] } {
>          lappend DEFAULT_VECTCFLAGS "-msse2"
>          if { [check_effective_target_sse2_runtime] } {
>              set dg-do-what-default run
>          } else {
> -            set dg-do-what-default compile
> +            set dg-do-what-default link
>          }
>      } elseif { [istarget mips*-*-*]
>                && [check_effective_target_nomips16] } {
> @@ -11681,7 +11681,7 @@ proc check_vect_support_and_set_flags { } {
>          if [check_effective_target_ultrasparc_hw] {
>              set dg-do-what-default run
>          } else {
> -            set dg-do-what-default compile
> +            set dg-do-what-default link
>          }
>      } elseif [istarget alpha*-*-*] {
>          # Alpha's vectorization capabilities are extremely limited.
> @@ -11694,7 +11694,7 @@ proc check_vect_support_and_set_flags { } {
>          if [check_alpha_max_hw_available] {
>              set dg-do-what-default run
>          } else {
> -            set dg-do-what-default compile
> +            set dg-do-what-default link
>          }
>      } elseif [istarget ia64-*-*] {
>          set dg-do-what-default run
> @@ -11707,7 +11707,7 @@ proc check_vect_support_and_set_flags { } {
>          if [is-effective-target arm_neon_hw] {
>              set dg-do-what-default run
>          } else {
> -            set dg-do-what-default compile
> +            set dg-do-what-default link
>          }
>      } elseif [istarget aarch64*-*-*] {
>          set dg-do-what-default run
> @@ -11731,7 +11731,7 @@ proc check_vect_support_and_set_flags { } {
>              set dg-do-what-default run
>          } else {
>             lappend DEFAULT_VECTCFLAGS "-march=z14" "-mzarch"
> -            set dg-do-what-default compile
> +            set dg-do-what-default link
>          }
>      } elseif [istarget amdgcn-*-*] {
>          set dg-do-what-default run
> @@ -11742,7 +11742,7 @@ proc check_vect_support_and_set_flags { } {
>             foreach item [add_options_for_riscv_v ""] {
>                 lappend DEFAULT_VECTCFLAGS $item
>             }
> -           set dg-do-what-default compile
> +           set dg-do-what-default link
>         }
>      } elseif [istarget loongarch*-*-*] {
>        # Set the default vectorization option to "-mlsx" due to the problem
> @@ -11751,7 +11751,7 @@ proc check_vect_support_and_set_flags { } {
>        if [check_effective_target_loongarch_sx_hw] {
>           set dg-do-what-default run
>        } else {
> -         set dg-do-what-default compile
> +         set dg-do-what-default link
>        }
>      } else {
>          return 0
>
>
> --
> Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
>    Free Software Activist                   GNU Toolchain Engineer
> More tolerance and less prejudice are key for inclusion and diversity
> Excluding neuro-others for not behaving ""normal"" is *not* inclusive

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] decay vect tests from run to link for pr95401
  2024-04-22 19:10     ` Richard Biener
@ 2024-04-29 23:30       ` Alexandre Oliva
  2024-04-30 12:39         ` Christophe Lyon
  0 siblings, 1 reply; 12+ messages in thread
From: Alexandre Oliva @ 2024-04-29 23:30 UTC (permalink / raw)
  To: Richard Biener
  Cc: GCC Patches, Rainer Orth, Mike Stump, David Edelsohn,
	Segher Boessenkool, Kewen Lin

On Apr 22, 2024, Richard Biener <richard.guenther@gmail.com> wrote:

>> Regstrapped on x86_64-linux-gnu and ppc64el-linux-gnu.  Also tested with
>> gcc-13 on ppc64-vx7r2 and ppc-vx7r2.  Ok to install?

> That makes sense.  OK

>> for  gcc/testsuite/ChangeLog
>> 
>> * lib/target-supports.exp (check_vect_support_and_set_flags):
>> Decay to link rather than compile.

Alas, linking may fail because of an incompatible libc, as reported by
Linaro with a link to their issue GNU-1206 (I'm not posting the link to
the fully-Javascrippled Jira web page; it shows nothing useful, and I
can't post feedback there) and to
https://ci.linaro.org/job/tcwg_gnu_embed_check_gcc--master-thumb_m7_hard_eabi-build/10/artifact/artifacts/00-sumfiles/
(where I could get useful information)

I'm reverting the patch, and I'll see about some alternate approach that
can accommodate this scenario after I return from LibrePlanet.

-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] decay vect tests from run to link for pr95401
  2024-04-29 23:30       ` Alexandre Oliva
@ 2024-04-30 12:39         ` Christophe Lyon
  2024-05-23 13:28           ` [PATCH] [testsuite] conditionalize dg-additional-sources on target and type Alexandre Oliva
  0 siblings, 1 reply; 12+ messages in thread
From: Christophe Lyon @ 2024-04-30 12:39 UTC (permalink / raw)
  To: Alexandre Oliva
  Cc: Richard Biener, GCC Patches, Rainer Orth, Mike Stump,
	David Edelsohn, Segher Boessenkool, Kewen Lin

Hi Alexandre,

On Tue, 30 Apr 2024 at 01:31, Alexandre Oliva <oliva@adacore.com> wrote:
>
> On Apr 22, 2024, Richard Biener <richard.guenther@gmail.com> wrote:
>
> >> Regstrapped on x86_64-linux-gnu and ppc64el-linux-gnu.  Also tested with
> >> gcc-13 on ppc64-vx7r2 and ppc-vx7r2.  Ok to install?
>
> > That makes sense.  OK
>
> >> for  gcc/testsuite/ChangeLog
> >>
> >> * lib/target-supports.exp (check_vect_support_and_set_flags):
> >> Decay to link rather than compile.
>
> Alas, linking may fail because of an incompatible libc, as reported by
> Linaro with a link to their issue GNU-1206 (I'm not posting the link to
> the fully-Javascrippled Jira web page; it shows nothing useful, and I
> can't post feedback there) and to
> https://ci.linaro.org/job/tcwg_gnu_embed_check_gcc--master-thumb_m7_hard_eabi-build/10/artifact/artifacts/00-sumfiles/
> (where I could get useful information)
>
> I'm reverting the patch, and I'll see about some alternate approach that
> can accommodate this scenario after I return from LibrePlanet.
>

Indeed, that's another instance of the tricky multilibs configuration issues.
In this case:
- we configure GCC: --disable-multilib --with-mode=thumb
--with-cpu=cortex-m7 --with-float=hard --target=arm-eabi
(we disable multilibs to save build time)
- we run the tests with
qemu/-mthumb/-march=armv7e-m+fp.dp/-mtune=cortex-m7/-mfloat-abi=hard/-mfpu=auto
which matches the GCC configuration flags,
but the vect.exp tests add -mfpu=neon -mfloat-abi=softfp -march=armv7-a
and link fails because the toolchain does not support softfp libs

HTH

Thanks,

Christophe

> --
> Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
>    Free Software Activist                   GNU Toolchain Engineer
> More tolerance and less prejudice are key for inclusion and diversity
> Excluding neuro-others for not behaving ""normal"" is *not* inclusive

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] [testsuite] conditionalize dg-additional-sources on target and type
  2024-04-30 12:39         ` Christophe Lyon
@ 2024-05-23 13:28           ` Alexandre Oliva
  2024-05-23 13:42             ` Christophe Lyon
  2024-05-29 20:11             ` Mike Stump
  0 siblings, 2 replies; 12+ messages in thread
From: Alexandre Oliva @ 2024-05-23 13:28 UTC (permalink / raw)
  To: Christophe Lyon
  Cc: Richard Biener, GCC Patches, Rainer Orth, Mike Stump,
	David Edelsohn, Segher Boessenkool, Kewen Lin

On Apr 30, 2024, Christophe Lyon <christophe.lyon@linaro.org> wrote:

> On Tue, 30 Apr 2024 at 01:31, Alexandre Oliva <oliva@adacore.com> wrote:
>> >> for  gcc/testsuite/ChangeLog
>> >>
>> >> * lib/target-supports.exp (check_vect_support_and_set_flags):
>> >> Decay to link rather than compile.
>> 
>> Alas, linking may fail because of an incompatible libc, as reported by
>> Linaro with a link to their issue GNU-1206 (I'm not posting the link to
>> the fully-Javascrippled Jira web page; it shows nothing useful, and I
>> can't post feedback there) and to
>> https://ci.linaro.org/job/tcwg_gnu_embed_check_gcc--master-thumb_m7_hard_eabi-build/10/artifact/artifacts/00-sumfiles/
>> (where I could get useful information)
>> 
>> I'm reverting the patch, and I'll see about some alternate approach

> Indeed, that's another instance of the tricky multilibs configuration issues.

> - we run the tests with
> qemu/-mthumb/-march=armv7e-m+fp.dp/-mtune=cortex-m7/-mfloat-abi=hard/-mfpu=auto
> which matches the GCC configuration flags,
> but the vect.exp tests add -mfpu=neon -mfloat-abi=softfp -march=armv7-a
> and link fails because the toolchain does not support softfp libs

Hello, Christophe, thanks for the info.

I came up with an entirely different approach:


g++.dg/vect/pr95401.cc has dg-additional-sources, and that fails when
check_vect_support_and_set_flags finds vector support lacking for
execution tests: tests decay to compile tests, and additional sources
are rejected by the compiler when compiling to a named output file.

At first I considered using some effective target to conditionalize
the additional sources.  There was no support for target-specific
additional sources, so I added that.

But then, I found that adding an effective target to check whether the
test involves linking would just make for busy work in this case, and
so I went ahead and adjusted the handling of additional sources to
refrain from adding them on compile tests, reporting them as
unsupported.

That solves the problem without using the newly-added machinery for
per-target additional sources, but I figured since I'd implemented it
I might as well contribute it, since there might be other uses for it.

Regstrapped on x86_64-linux-gnu.  Also tested on ppc64-vx7r2 with
gcc-13.  Ok to install?


for  gcc/ChangeLog

	* doc/sourcebuild.texi (dg-additional-sources): Document
	newly-added support for target selectors, and implicit discard
	on non-linking tests that name the compiler output explicitly.

for  gcc/testsuite/ChangeLog

	* lib/gcc-defs.exp (dg-additional-sources): Support target
	selectors.  Make it cumulative.
	(dg-additional-files-options): Take dest and type.  Note
	unsupported additional sources when not linking and naming the
	compiler output.  Adjust source dirname prepending to cope
	with leading blanks.
	* lib/g++.exp (g++_target_compile): Pass dest and type on to
	dg-additional-files-options.
	* lib/gcc.exp (gcc_target_compile): Likewise.
	* lib/gdc.exp (gdb_target_compile): Likewise.
	* lib/gfortran.exp (gfortran_target_compile): Likewise.
	* lib/go.exp (go_target_compile): Likewise.
	* lib/obj-c++.exp (obj-c++_target_compile): Likewise.
	* lib/objc.exp (objc_target_compile): Likewise.
	* lib/rust.exp (rust_target_compile): Likewise.
	* lib/profopt.exp (profopt-execute): Likewise-ish.
---
 gcc/doc/sourcebuild.texi       |    8 +++++++-
 gcc/testsuite/lib/g++.exp      |    2 +-
 gcc/testsuite/lib/gcc-defs.exp |   35 ++++++++++++++++++++++++++++++-----
 gcc/testsuite/lib/gcc.exp      |    2 +-
 gcc/testsuite/lib/gdc.exp      |    2 +-
 gcc/testsuite/lib/gfortran.exp |    2 +-
 gcc/testsuite/lib/go.exp       |    2 +-
 gcc/testsuite/lib/obj-c++.exp  |    2 +-
 gcc/testsuite/lib/objc.exp     |    2 +-
 gcc/testsuite/lib/profopt.exp  |    2 +-
 gcc/testsuite/lib/rust.exp     |    2 +-
 11 files changed, 46 insertions(+), 15 deletions(-)

diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
index 8e4e59ac44c74..e997dbec3334b 100644
--- a/gcc/doc/sourcebuild.texi
+++ b/gcc/doc/sourcebuild.texi
@@ -1320,9 +1320,15 @@ to @var{var_value} before execution of the program created by the test.
 Specify additional files, other than source files, that must be copied
 to the system where the compiler runs.
 
-@item @{ dg-additional-sources "@var{filelist}" @}
+@item @{ dg-additional-sources "@var{filelist}" [@{ target @var{selector} @}] @}
 Specify additional source files to appear in the compile line
 following the main test file.
+If the directive includes the optional @samp{@{ @var{selector} @}}
+then the additional sources are only added if the target system
+matches the @var{selector}.
+Additional sources are generally used only in @samp{link} and @samp{run}
+tests; they are reported as unsupported and discarded in other kinds of
+tests that direct the compiler to output to a single file.
 @end table
 
 @subsubsection Add checks at the end of a test
diff --git a/gcc/testsuite/lib/g++.exp b/gcc/testsuite/lib/g++.exp
index 0e47769c25b8b..a6b34d5d3a2b7 100644
--- a/gcc/testsuite/lib/g++.exp
+++ b/gcc/testsuite/lib/g++.exp
@@ -326,7 +326,7 @@ proc g++_target_compile { source dest type options } {
         append board_info($tboard,multilib_flags) " $flags_to_postpone"
     }
 
-    set options [dg-additional-files-options $options $source]
+    set options [dg-additional-files-options $options $source $dest $type]
 
     if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } {
 	lappend options "libs=${gluefile}"
diff --git a/gcc/testsuite/lib/gcc-defs.exp b/gcc/testsuite/lib/gcc-defs.exp
index 70215ed49052e..cdca4c254d6ec 100644
--- a/gcc/testsuite/lib/gcc-defs.exp
+++ b/gcc/testsuite/lib/gcc-defs.exp
@@ -307,7 +307,22 @@ set additional_sources_used ""
 
 proc dg-additional-sources { args } {
     global additional_sources
-    set additional_sources [lindex $args 1]
+
+    if { [llength $args] > 3 } {
+	error "[lindex $args 0]: too many arguments"
+	return
+    }
+
+    if { [llength $args] >= 3 } {
+	switch [dg-process-target [lindex $args 2]] {
+	    "S" { append additional_sources " [lindex $args 1]" }
+	    "N" { }
+	    "F" { error "[lindex $args 0]: `xfail' not allowed here" }
+	    "P" { error "[lindex $args 0]: `xfail' not allowed here" }
+	}
+    } else {
+	append additional_sources " [lindex $args 1]"
+    }
 }
 
 # Record additional files -- other than source files -- that must be
@@ -383,20 +398,30 @@ proc gcc_adjust_linker_flags {} {
 
 # Return an updated version of OPTIONS that mentions any additional
 # source files registered with dg-additional-sources.  SOURCE is the
-# name of the test case.
+# name of the test case.  If DEST is given and TYPE does not require
+# linking, additional sources are noted as unsupported rather than
+# added, because the compiler rejects a single output for multiple
+# sources.
 
-proc dg-additional-files-options { options source } {
+proc dg-additional-files-options { options source dest type } {
     gcc_adjust_linker_flags
 
     global additional_sources
     global additional_sources_used
     global additional_files
     set to_download [list]
-    if { $additional_sources != "" } then {
+    if { $additional_sources == "" } then {
+    } elseif { $type != "executable" && $dest != "" } then {
+	foreach s $additional_sources {
+	    unsupported "$s: additional-source will not be used to build $dest"
+	}
+	set additional_sources_used ""
+	set additional_sources ""
+    } else {
 	if [is_remote host] {
 	    lappend options "additional_flags=$additional_sources"
 	}
-	regsub -all "^| " $additional_sources " [file dirname $source]/" additional_sources
+	regsub -all {^ *|  *} $additional_sources " [file dirname $source]/" additional_sources
 	if ![is_remote host] {
 	    lappend options "additional_flags=$additional_sources"
 	}
diff --git a/gcc/testsuite/lib/gcc.exp b/gcc/testsuite/lib/gcc.exp
index 63e61c3c9fd9c..3bcd98eb9a7ba 100644
--- a/gcc/testsuite/lib/gcc.exp
+++ b/gcc/testsuite/lib/gcc.exp
@@ -159,7 +159,7 @@ proc gcc_target_compile { source dest type options } {
 
     lappend options "timeout=[timeout_value]"
     lappend options "compiler=$GCC_UNDER_TEST"
-    set options [dg-additional-files-options $options $source]
+    set options [dg-additional-files-options $options $source $dest $type]
 
     if {[target_info needs_status_wrapper] != "" && \
 	    [target_info needs_status_wrapper] != "0" && \
diff --git a/gcc/testsuite/lib/gdc.exp b/gcc/testsuite/lib/gdc.exp
index 3c284229609a9..fd9cea67d0a9f 100644
--- a/gcc/testsuite/lib/gdc.exp
+++ b/gcc/testsuite/lib/gdc.exp
@@ -327,6 +327,6 @@ proc gdc_target_compile { source dest type options } {
     lappend options "compiler=$GDC_UNDER_TEST"
 
     set options [concat "$always_dflags" $options]
-    set options [dg-additional-files-options $options $source]
+    set options [dg-additional-files-options $options $source $dest $type]
     return [target_compile $source $dest $type $options]
 }
diff --git a/gcc/testsuite/lib/gfortran.exp b/gcc/testsuite/lib/gfortran.exp
index 1ccb81ccec5a8..3a9e81b69a3d1 100644
--- a/gcc/testsuite/lib/gfortran.exp
+++ b/gcc/testsuite/lib/gfortran.exp
@@ -295,7 +295,7 @@ proc gfortran_target_compile { source dest type options } {
     lappend options "timeout=[timeout_value]"
 
     set options [concat "$ALWAYS_GFORTRANFLAGS" $options]
-    set options [dg-additional-files-options $options $source]
+    set options [dg-additional-files-options $options $source $dest $type]
     set return_val [target_compile $source $dest $type $options]
 
     if {[board_info $tboard exists multilib_flags]} {
diff --git a/gcc/testsuite/lib/go.exp b/gcc/testsuite/lib/go.exp
index 714afb173d8e0..509d528d861fc 100644
--- a/gcc/testsuite/lib/go.exp
+++ b/gcc/testsuite/lib/go.exp
@@ -221,6 +221,6 @@ proc go_target_compile { source dest type options } {
     lappend options "compiler=$GOC_UNDER_TEST"
 
     set options [concat "$ALWAYS_GOCFLAGS" $options]
-    set options [dg-additional-files-options $options $source]
+    set options [dg-additional-files-options $options $source $dest $type]
     return [target_compile $source $dest $type $options]
 }
diff --git a/gcc/testsuite/lib/obj-c++.exp b/gcc/testsuite/lib/obj-c++.exp
index 854dc264f9d0e..1c5ef57ce8370 100644
--- a/gcc/testsuite/lib/obj-c++.exp
+++ b/gcc/testsuite/lib/obj-c++.exp
@@ -388,7 +388,7 @@ proc obj-c++_target_compile { source dest type options } {
 
     set options [concat "$ALWAYS_OBJCXXFLAGS" $options];
 
-    set options [dg-additional-files-options $options $source]
+    set options [dg-additional-files-options $options $source $dest $type]
 
     set result [target_compile $source $dest $type $options]
 
diff --git a/gcc/testsuite/lib/objc.exp b/gcc/testsuite/lib/objc.exp
index b02fd77289c45..3d2cc05c15d11 100644
--- a/gcc/testsuite/lib/objc.exp
+++ b/gcc/testsuite/lib/objc.exp
@@ -248,7 +248,7 @@ proc objc_target_compile { source dest type options } {
 
     set_ld_library_path_env_vars
 
-    set options [dg-additional-files-options $options $source]
+    set options [dg-additional-files-options $options $source $dest $type]
 
     return [target_compile $source $dest $type $options]
 }
diff --git a/gcc/testsuite/lib/profopt.exp b/gcc/testsuite/lib/profopt.exp
index 6524d95f69d5a..6d7159b7a8691 100644
--- a/gcc/testsuite/lib/profopt.exp
+++ b/gcc/testsuite/lib/profopt.exp
@@ -388,7 +388,7 @@ proc profopt-execute { src } {
 	# schedule removal of dump files et al
 	# Do this before the call below destroys additional_sources..
 	append use_final_code [schedule-cleanups "$option $extra_flags"]
-        set extra_options [dg-additional-files-options "" "$src"]
+        set extra_options [dg-additional-files-options "" "$src" $execname1 "executable"]
 
 	# Remove old profiling data files.  Make sure additional_sources_used is
 	# valid, by running it after dg-additional-files-options.
diff --git a/gcc/testsuite/lib/rust.exp b/gcc/testsuite/lib/rust.exp
index 5ded0edf91464..4c296228fa2af 100644
--- a/gcc/testsuite/lib/rust.exp
+++ b/gcc/testsuite/lib/rust.exp
@@ -182,7 +182,7 @@ proc rust_target_compile { source dest type options } {
     lappend options "compiler=$RUST_UNDER_TEST"
 
     set options [concat "$ALWAYS_RUSTFLAGS" $options]
-    set options [dg-additional-files-options $options $source]
+    set options [dg-additional-files-options $options $source $dest $type]
 
     return [target_compile $source $dest $type $options]
 }


-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] [testsuite] conditionalize dg-additional-sources on target and type
  2024-05-23 13:28           ` [PATCH] [testsuite] conditionalize dg-additional-sources on target and type Alexandre Oliva
@ 2024-05-23 13:42             ` Christophe Lyon
  2024-05-29 20:11             ` Mike Stump
  1 sibling, 0 replies; 12+ messages in thread
From: Christophe Lyon @ 2024-05-23 13:42 UTC (permalink / raw)
  To: Alexandre Oliva
  Cc: Richard Biener, GCC Patches, Rainer Orth, Mike Stump,
	David Edelsohn, Segher Boessenkool, Kewen Lin

Hi Alexandre,


On Thu, 23 May 2024 at 15:29, Alexandre Oliva <oliva@adacore.com> wrote:
>
> On Apr 30, 2024, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>
> > On Tue, 30 Apr 2024 at 01:31, Alexandre Oliva <oliva@adacore.com> wrote:
> >> >> for  gcc/testsuite/ChangeLog
> >> >>
> >> >> * lib/target-supports.exp (check_vect_support_and_set_flags):
> >> >> Decay to link rather than compile.
> >>
> >> Alas, linking may fail because of an incompatible libc, as reported by
> >> Linaro with a link to their issue GNU-1206 (I'm not posting the link to
> >> the fully-Javascrippled Jira web page; it shows nothing useful, and I
> >> can't post feedback there) and to
> >> https://ci.linaro.org/job/tcwg_gnu_embed_check_gcc--master-thumb_m7_hard_eabi-build/10/artifact/artifacts/00-sumfiles/
> >> (where I could get useful information)
> >>
> >> I'm reverting the patch, and I'll see about some alternate approach
>
> > Indeed, that's another instance of the tricky multilibs configuration issues.
>
> > - we run the tests with
> > qemu/-mthumb/-march=armv7e-m+fp.dp/-mtune=cortex-m7/-mfloat-abi=hard/-mfpu=auto
> > which matches the GCC configuration flags,
> > but the vect.exp tests add -mfpu=neon -mfloat-abi=softfp -march=armv7-a
> > and link fails because the toolchain does not support softfp libs
>
> Hello, Christophe, thanks for the info.
>
> I came up with an entirely different approach:
>
>
> g++.dg/vect/pr95401.cc has dg-additional-sources, and that fails when
> check_vect_support_and_set_flags finds vector support lacking for
> execution tests: tests decay to compile tests, and additional sources
> are rejected by the compiler when compiling to a named output file.
>
> At first I considered using some effective target to conditionalize
> the additional sources.  There was no support for target-specific
> additional sources, so I added that.
>
> But then, I found that adding an effective target to check whether the
> test involves linking would just make for busy work in this case, and
> so I went ahead and adjusted the handling of additional sources to
> refrain from adding them on compile tests, reporting them as
> unsupported.
>
> That solves the problem without using the newly-added machinery for
> per-target additional sources, but I figured since I'd implemented it
> I might as well contribute it, since there might be other uses for it.

Thanks for improving this, LGTM at quick glance, but I can't approve :-)

Christophe

>
> Regstrapped on x86_64-linux-gnu.  Also tested on ppc64-vx7r2 with
> gcc-13.  Ok to install?
>
>
> for  gcc/ChangeLog
>
>         * doc/sourcebuild.texi (dg-additional-sources): Document
>         newly-added support for target selectors, and implicit discard
>         on non-linking tests that name the compiler output explicitly.
>
> for  gcc/testsuite/ChangeLog
>
>         * lib/gcc-defs.exp (dg-additional-sources): Support target
>         selectors.  Make it cumulative.
>         (dg-additional-files-options): Take dest and type.  Note
>         unsupported additional sources when not linking and naming the
>         compiler output.  Adjust source dirname prepending to cope
>         with leading blanks.
>         * lib/g++.exp (g++_target_compile): Pass dest and type on to
>         dg-additional-files-options.
>         * lib/gcc.exp (gcc_target_compile): Likewise.
>         * lib/gdc.exp (gdb_target_compile): Likewise.
>         * lib/gfortran.exp (gfortran_target_compile): Likewise.
>         * lib/go.exp (go_target_compile): Likewise.
>         * lib/obj-c++.exp (obj-c++_target_compile): Likewise.
>         * lib/objc.exp (objc_target_compile): Likewise.
>         * lib/rust.exp (rust_target_compile): Likewise.
>         * lib/profopt.exp (profopt-execute): Likewise-ish.
> ---
>  gcc/doc/sourcebuild.texi       |    8 +++++++-
>  gcc/testsuite/lib/g++.exp      |    2 +-
>  gcc/testsuite/lib/gcc-defs.exp |   35 ++++++++++++++++++++++++++++++-----
>  gcc/testsuite/lib/gcc.exp      |    2 +-
>  gcc/testsuite/lib/gdc.exp      |    2 +-
>  gcc/testsuite/lib/gfortran.exp |    2 +-
>  gcc/testsuite/lib/go.exp       |    2 +-
>  gcc/testsuite/lib/obj-c++.exp  |    2 +-
>  gcc/testsuite/lib/objc.exp     |    2 +-
>  gcc/testsuite/lib/profopt.exp  |    2 +-
>  gcc/testsuite/lib/rust.exp     |    2 +-
>  11 files changed, 46 insertions(+), 15 deletions(-)
>
> diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
> index 8e4e59ac44c74..e997dbec3334b 100644
> --- a/gcc/doc/sourcebuild.texi
> +++ b/gcc/doc/sourcebuild.texi
> @@ -1320,9 +1320,15 @@ to @var{var_value} before execution of the program created by the test.
>  Specify additional files, other than source files, that must be copied
>  to the system where the compiler runs.
>
> -@item @{ dg-additional-sources "@var{filelist}" @}
> +@item @{ dg-additional-sources "@var{filelist}" [@{ target @var{selector} @}] @}
>  Specify additional source files to appear in the compile line
>  following the main test file.
> +If the directive includes the optional @samp{@{ @var{selector} @}}
> +then the additional sources are only added if the target system
> +matches the @var{selector}.
> +Additional sources are generally used only in @samp{link} and @samp{run}
> +tests; they are reported as unsupported and discarded in other kinds of
> +tests that direct the compiler to output to a single file.
>  @end table
>
>  @subsubsection Add checks at the end of a test
> diff --git a/gcc/testsuite/lib/g++.exp b/gcc/testsuite/lib/g++.exp
> index 0e47769c25b8b..a6b34d5d3a2b7 100644
> --- a/gcc/testsuite/lib/g++.exp
> +++ b/gcc/testsuite/lib/g++.exp
> @@ -326,7 +326,7 @@ proc g++_target_compile { source dest type options } {
>          append board_info($tboard,multilib_flags) " $flags_to_postpone"
>      }
>
> -    set options [dg-additional-files-options $options $source]
> +    set options [dg-additional-files-options $options $source $dest $type]
>
>      if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } {
>         lappend options "libs=${gluefile}"
> diff --git a/gcc/testsuite/lib/gcc-defs.exp b/gcc/testsuite/lib/gcc-defs.exp
> index 70215ed49052e..cdca4c254d6ec 100644
> --- a/gcc/testsuite/lib/gcc-defs.exp
> +++ b/gcc/testsuite/lib/gcc-defs.exp
> @@ -307,7 +307,22 @@ set additional_sources_used ""
>
>  proc dg-additional-sources { args } {
>      global additional_sources
> -    set additional_sources [lindex $args 1]
> +
> +    if { [llength $args] > 3 } {
> +       error "[lindex $args 0]: too many arguments"
> +       return
> +    }
> +
> +    if { [llength $args] >= 3 } {
> +       switch [dg-process-target [lindex $args 2]] {
> +           "S" { append additional_sources " [lindex $args 1]" }
> +           "N" { }
> +           "F" { error "[lindex $args 0]: `xfail' not allowed here" }
> +           "P" { error "[lindex $args 0]: `xfail' not allowed here" }
> +       }
> +    } else {
> +       append additional_sources " [lindex $args 1]"
> +    }
>  }
>
>  # Record additional files -- other than source files -- that must be
> @@ -383,20 +398,30 @@ proc gcc_adjust_linker_flags {} {
>
>  # Return an updated version of OPTIONS that mentions any additional
>  # source files registered with dg-additional-sources.  SOURCE is the
> -# name of the test case.
> +# name of the test case.  If DEST is given and TYPE does not require
> +# linking, additional sources are noted as unsupported rather than
> +# added, because the compiler rejects a single output for multiple
> +# sources.
>
> -proc dg-additional-files-options { options source } {
> +proc dg-additional-files-options { options source dest type } {
>      gcc_adjust_linker_flags
>
>      global additional_sources
>      global additional_sources_used
>      global additional_files
>      set to_download [list]
> -    if { $additional_sources != "" } then {
> +    if { $additional_sources == "" } then {
> +    } elseif { $type != "executable" && $dest != "" } then {
> +       foreach s $additional_sources {
> +           unsupported "$s: additional-source will not be used to build $dest"
> +       }
> +       set additional_sources_used ""
> +       set additional_sources ""
> +    } else {
>         if [is_remote host] {
>             lappend options "additional_flags=$additional_sources"
>         }
> -       regsub -all "^| " $additional_sources " [file dirname $source]/" additional_sources
> +       regsub -all {^ *|  *} $additional_sources " [file dirname $source]/" additional_sources
>         if ![is_remote host] {
>             lappend options "additional_flags=$additional_sources"
>         }
> diff --git a/gcc/testsuite/lib/gcc.exp b/gcc/testsuite/lib/gcc.exp
> index 63e61c3c9fd9c..3bcd98eb9a7ba 100644
> --- a/gcc/testsuite/lib/gcc.exp
> +++ b/gcc/testsuite/lib/gcc.exp
> @@ -159,7 +159,7 @@ proc gcc_target_compile { source dest type options } {
>
>      lappend options "timeout=[timeout_value]"
>      lappend options "compiler=$GCC_UNDER_TEST"
> -    set options [dg-additional-files-options $options $source]
> +    set options [dg-additional-files-options $options $source $dest $type]
>
>      if {[target_info needs_status_wrapper] != "" && \
>             [target_info needs_status_wrapper] != "0" && \
> diff --git a/gcc/testsuite/lib/gdc.exp b/gcc/testsuite/lib/gdc.exp
> index 3c284229609a9..fd9cea67d0a9f 100644
> --- a/gcc/testsuite/lib/gdc.exp
> +++ b/gcc/testsuite/lib/gdc.exp
> @@ -327,6 +327,6 @@ proc gdc_target_compile { source dest type options } {
>      lappend options "compiler=$GDC_UNDER_TEST"
>
>      set options [concat "$always_dflags" $options]
> -    set options [dg-additional-files-options $options $source]
> +    set options [dg-additional-files-options $options $source $dest $type]
>      return [target_compile $source $dest $type $options]
>  }
> diff --git a/gcc/testsuite/lib/gfortran.exp b/gcc/testsuite/lib/gfortran.exp
> index 1ccb81ccec5a8..3a9e81b69a3d1 100644
> --- a/gcc/testsuite/lib/gfortran.exp
> +++ b/gcc/testsuite/lib/gfortran.exp
> @@ -295,7 +295,7 @@ proc gfortran_target_compile { source dest type options } {
>      lappend options "timeout=[timeout_value]"
>
>      set options [concat "$ALWAYS_GFORTRANFLAGS" $options]
> -    set options [dg-additional-files-options $options $source]
> +    set options [dg-additional-files-options $options $source $dest $type]
>      set return_val [target_compile $source $dest $type $options]
>
>      if {[board_info $tboard exists multilib_flags]} {
> diff --git a/gcc/testsuite/lib/go.exp b/gcc/testsuite/lib/go.exp
> index 714afb173d8e0..509d528d861fc 100644
> --- a/gcc/testsuite/lib/go.exp
> +++ b/gcc/testsuite/lib/go.exp
> @@ -221,6 +221,6 @@ proc go_target_compile { source dest type options } {
>      lappend options "compiler=$GOC_UNDER_TEST"
>
>      set options [concat "$ALWAYS_GOCFLAGS" $options]
> -    set options [dg-additional-files-options $options $source]
> +    set options [dg-additional-files-options $options $source $dest $type]
>      return [target_compile $source $dest $type $options]
>  }
> diff --git a/gcc/testsuite/lib/obj-c++.exp b/gcc/testsuite/lib/obj-c++.exp
> index 854dc264f9d0e..1c5ef57ce8370 100644
> --- a/gcc/testsuite/lib/obj-c++.exp
> +++ b/gcc/testsuite/lib/obj-c++.exp
> @@ -388,7 +388,7 @@ proc obj-c++_target_compile { source dest type options } {
>
>      set options [concat "$ALWAYS_OBJCXXFLAGS" $options];
>
> -    set options [dg-additional-files-options $options $source]
> +    set options [dg-additional-files-options $options $source $dest $type]
>
>      set result [target_compile $source $dest $type $options]
>
> diff --git a/gcc/testsuite/lib/objc.exp b/gcc/testsuite/lib/objc.exp
> index b02fd77289c45..3d2cc05c15d11 100644
> --- a/gcc/testsuite/lib/objc.exp
> +++ b/gcc/testsuite/lib/objc.exp
> @@ -248,7 +248,7 @@ proc objc_target_compile { source dest type options } {
>
>      set_ld_library_path_env_vars
>
> -    set options [dg-additional-files-options $options $source]
> +    set options [dg-additional-files-options $options $source $dest $type]
>
>      return [target_compile $source $dest $type $options]
>  }
> diff --git a/gcc/testsuite/lib/profopt.exp b/gcc/testsuite/lib/profopt.exp
> index 6524d95f69d5a..6d7159b7a8691 100644
> --- a/gcc/testsuite/lib/profopt.exp
> +++ b/gcc/testsuite/lib/profopt.exp
> @@ -388,7 +388,7 @@ proc profopt-execute { src } {
>         # schedule removal of dump files et al
>         # Do this before the call below destroys additional_sources..
>         append use_final_code [schedule-cleanups "$option $extra_flags"]
> -        set extra_options [dg-additional-files-options "" "$src"]
> +        set extra_options [dg-additional-files-options "" "$src" $execname1 "executable"]
>
>         # Remove old profiling data files.  Make sure additional_sources_used is
>         # valid, by running it after dg-additional-files-options.
> diff --git a/gcc/testsuite/lib/rust.exp b/gcc/testsuite/lib/rust.exp
> index 5ded0edf91464..4c296228fa2af 100644
> --- a/gcc/testsuite/lib/rust.exp
> +++ b/gcc/testsuite/lib/rust.exp
> @@ -182,7 +182,7 @@ proc rust_target_compile { source dest type options } {
>      lappend options "compiler=$RUST_UNDER_TEST"
>
>      set options [concat "$ALWAYS_RUSTFLAGS" $options]
> -    set options [dg-additional-files-options $options $source]
> +    set options [dg-additional-files-options $options $source $dest $type]
>
>      return [target_compile $source $dest $type $options]
>  }
>
>
> --
> Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
>    Free Software Activist                   GNU Toolchain Engineer
> More tolerance and less prejudice are key for inclusion and diversity
> Excluding neuro-others for not behaving ""normal"" is *not* inclusive

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] [testsuite] conditionalize dg-additional-sources on target and type
  2024-05-23 13:28           ` [PATCH] [testsuite] conditionalize dg-additional-sources on target and type Alexandre Oliva
  2024-05-23 13:42             ` Christophe Lyon
@ 2024-05-29 20:11             ` Mike Stump
  1 sibling, 0 replies; 12+ messages in thread
From: Mike Stump @ 2024-05-29 20:11 UTC (permalink / raw)
  To: Alexandre Oliva
  Cc: Christophe Lyon, Richard Biener, GCC Patches, Rainer Orth,
	David Edelsohn, Segher Boessenkool, Kewen Lin

On May 23, 2024, at 6:28 AM, Alexandre Oliva <oliva@adacore.com> wrote;
> I came up with an entirely different approach:
> 
> 
> g++.dg/vect/pr95401.cc has dg-additional-sources, and that fails when
> check_vect_support_and_set_flags finds vector support lacking for
> execution tests: tests decay to compile tests, and additional sources
> are rejected by the compiler when compiling to a named output file.
> 
> At first I considered using some effective target to conditionalize
> the additional sources.  There was no support for target-specific
> additional sources, so I added that.
> 
> But then, I found that adding an effective target to check whether the
> test involves linking would just make for busy work in this case, and
> so I went ahead and adjusted the handling of additional sources to
> refrain from adding them on compile tests, reporting them as
> unsupported.
> 
> That solves the problem without using the newly-added machinery for
> per-target additional sources, but I figured since I'd implemented it
> I might as well contribute it, since there might be other uses for it.
> 
> Regstrapped on x86_64-linux-gnu.  Also tested on ppc64-vx7r2 with
> gcc-13.  Ok to install?

Ok.


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-05-29 20:11 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-11  7:38 [RFC] decay vect tests from run to link for pr95401 Alexandre Oliva
2021-03-11  9:48 ` Richard Biener
2021-03-11 12:17   ` Richard Sandiford
2021-03-11 14:47   ` Alexandre Oliva
2021-03-12  7:42     ` Richard Biener
2024-04-22 10:05   ` [PATCH] " Alexandre Oliva
2024-04-22 19:10     ` Richard Biener
2024-04-29 23:30       ` Alexandre Oliva
2024-04-30 12:39         ` Christophe Lyon
2024-05-23 13:28           ` [PATCH] [testsuite] conditionalize dg-additional-sources on target and type Alexandre Oliva
2024-05-23 13:42             ` Christophe Lyon
2024-05-29 20:11             ` Mike Stump

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).