Message ID or34wddhnw.fsf_-_@lxoliva.fsfla.org
State Committed
Commit 5a9e8b0cbbc1a10d73f1809e76bfec73a4386be3
Headers
Series |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Testing passed

Commit Message

Alexandre Oliva Dec. 7, 2023, 5:52 p.m. UTC
  On Dec  7, 2023, Thomas Schwinge <thomas@codesourcery.com> wrote:

> Thank you for looking into this so promptly!

You're welcome ;-)


>     during IPA pass: emutls
>     [...]/source-gcc/gcc/testsuite/c-c++-common/strub-unsupported-3.c:18:1: internal compiler error: in verify_curr_properties, at passes.cc:2198

Aah, this smells a lot like the issue that François-Xavier reported,
that the following patch is expected to fix.  I'm still regstrapping it
on x86_64-linux-gnu, after checking that it addressed the symptom on a
cross compiler to the target for which it had originally been reported.
Ok to install, once you confirm that it cures these ICEs?


strub: skip emutls after strubm errors

The emutls pass requires PROP_ssa, but if the strubm pass (or any
other pre-SSA pass) issues errors, all of the build_ssa_passes are
skipped, so the property is not set, but emutls still attempts to run,
on targets that use it, despite earlier errors, so it hits the
unsatisfied requirement.

Adjust emutls to be skipped in case of earlier errors.


for  gcc/ChangeLog

	* tree-emutls.cc: Include diagnostic-core.h.
	(pass_ipa_lower_emutls::gate): Skip if errors were seen.
---
 gcc/tree-emutls.cc |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

Comments

Richard Biener Dec. 8, 2023, 6:46 a.m. UTC | #1
On Thu, Dec 7, 2023 at 6:52 PM Alexandre Oliva <oliva@adacore.com> wrote:
>
> On Dec  7, 2023, Thomas Schwinge <thomas@codesourcery.com> wrote:
>
> > Thank you for looking into this so promptly!
>
> You're welcome ;-)
>
>
> >     during IPA pass: emutls
> >     [...]/source-gcc/gcc/testsuite/c-c++-common/strub-unsupported-3.c:18:1: internal compiler error: in verify_curr_properties, at passes.cc:2198
>
> Aah, this smells a lot like the issue that François-Xavier reported,
> that the following patch is expected to fix.  I'm still regstrapping it
> on x86_64-linux-gnu, after checking that it addressed the symptom on a
> cross compiler to the target for which it had originally been reported.
> Ok to install, once you confirm that it cures these ICEs?
>
>
> strub: skip emutls after strubm errors
>
> The emutls pass requires PROP_ssa, but if the strubm pass (or any
> other pre-SSA pass) issues errors, all of the build_ssa_passes are
> skipped, so the property is not set, but emutls still attempts to run,
> on targets that use it, despite earlier errors, so it hits the
> unsatisfied requirement.
>
> Adjust emutls to be skipped in case of earlier errors.

OK.

>
> for  gcc/ChangeLog
>
>         * tree-emutls.cc: Include diagnostic-core.h.
>         (pass_ipa_lower_emutls::gate): Skip if errors were seen.
> ---
>  gcc/tree-emutls.cc |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/tree-emutls.cc b/gcc/tree-emutls.cc
> index 5dca5a8291356..38de202717a1a 100644
> --- a/gcc/tree-emutls.cc
> +++ b/gcc/tree-emutls.cc
> @@ -35,6 +35,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "langhooks.h"
>  #include "tree-iterator.h"
>  #include "gimplify.h"
> +#include "diagnostic-core.h" /* for seen_error */
>
>  /* Whenever a target does not support thread-local storage (TLS) natively,
>     we can emulate it with some run-time support in libgcc.  This will in
> @@ -841,7 +842,7 @@ public:
>    bool gate (function *) final override
>      {
>        /* If the target supports TLS natively, we need do nothing here.  */
> -      return !targetm.have_tls;
> +      return !targetm.have_tls && !seen_error ();
>      }
>
>    unsigned int execute (function *) final override
>
>
> --
> 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
  
Thomas Schwinge Dec. 8, 2023, 9:33 a.m. UTC | #2
Hi Alexandre!

On 2023-12-07T14:52:19-0300, Alexandre Oliva <oliva@adacore.com> wrote:
> On Dec  7, 2023, Thomas Schwinge <thomas@codesourcery.com> wrote:
>>     during IPA pass: emutls
>>     [...]/source-gcc/gcc/testsuite/c-c++-common/strub-unsupported-3.c:18:1: internal compiler error: in verify_curr_properties, at passes.cc:2198
>
> Aah, this smells a lot like the issue that François-Xavier reported,
> that the following patch is expected to fix.  I'm still regstrapping it
> on x86_64-linux-gnu, after checking that it addressed the symptom on a
> cross compiler to the target for which it had originally been reported.
> Ok to install, once you confirm that it cures these ICEs?

Yes, GCC/nvptx ICEs gone with that, thanks!


Grüße
 Thomas


> strub: skip emutls after strubm errors
>
> The emutls pass requires PROP_ssa, but if the strubm pass (or any
> other pre-SSA pass) issues errors, all of the build_ssa_passes are
> skipped, so the property is not set, but emutls still attempts to run,
> on targets that use it, despite earlier errors, so it hits the
> unsatisfied requirement.
>
> Adjust emutls to be skipped in case of earlier errors.
>
>
> for  gcc/ChangeLog
>
>       * tree-emutls.cc: Include diagnostic-core.h.
>       (pass_ipa_lower_emutls::gate): Skip if errors were seen.
> ---
>  gcc/tree-emutls.cc |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/tree-emutls.cc b/gcc/tree-emutls.cc
> index 5dca5a8291356..38de202717a1a 100644
> --- a/gcc/tree-emutls.cc
> +++ b/gcc/tree-emutls.cc
> @@ -35,6 +35,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "langhooks.h"
>  #include "tree-iterator.h"
>  #include "gimplify.h"
> +#include "diagnostic-core.h" /* for seen_error */
>
>  /* Whenever a target does not support thread-local storage (TLS) natively,
>     we can emulate it with some run-time support in libgcc.  This will in
> @@ -841,7 +842,7 @@ public:
>    bool gate (function *) final override
>      {
>        /* If the target supports TLS natively, we need do nothing here.  */
> -      return !targetm.have_tls;
> +      return !targetm.have_tls && !seen_error ();
>      }
>
>    unsigned int execute (function *) final override
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
  
FX Coudert Dec. 10, 2023, 9:16 a.m. UTC | #3
> Yes, GCC/nvptx ICEs gone with that, thanks!

And on darwin as well, test results are back to the same state as before. Thanks!

FX
  

Patch

diff --git a/gcc/tree-emutls.cc b/gcc/tree-emutls.cc
index 5dca5a8291356..38de202717a1a 100644
--- a/gcc/tree-emutls.cc
+++ b/gcc/tree-emutls.cc
@@ -35,6 +35,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "langhooks.h"
 #include "tree-iterator.h"
 #include "gimplify.h"
+#include "diagnostic-core.h" /* for seen_error */
 
 /* Whenever a target does not support thread-local storage (TLS) natively,
    we can emulate it with some run-time support in libgcc.  This will in
@@ -841,7 +842,7 @@  public:
   bool gate (function *) final override
     {
       /* If the target supports TLS natively, we need do nothing here.  */
-      return !targetm.have_tls;
+      return !targetm.have_tls && !seen_error ();
     }
 
   unsigned int execute (function *) final override