* [PATCH, MIPS]: Fix internal compiler error: in check_bool_attrs, at recog.c:2218 for micromips attribute
@ 2015-05-14 14:51 Robert Suchanek
2015-05-18 11:15 ` Matthew Fortune
0 siblings, 1 reply; 7+ messages in thread
From: Robert Suchanek @ 2015-05-14 14:51 UTC (permalink / raw)
To: Matthew Fortune, Catherine_Moore; +Cc: gcc-patches
Hi,
This patch fixes an internal compiler error when micromips/nomicromips
attributes are used.
The problem here was that the cached boolean attributes for the current
target did not agree with the uncached attributes throwing an assertion error.
It appears that saving and restoring the state for micromips was missing,
just like there is for mips16. OK to apply?
Regards,
Robert
gcc/
* config/mips/mips.c (micromips_globals): New variable.
(mips_set_compression_mode): Save and reinitialize target-dependent
state for microMIPS.
gcc/testsuite
* gcc.target/mips/umips-attr.c: New test.
---
gcc/config/mips/mips.c | 10 ++++++++++
gcc/testsuite/gcc.target/mips/umips-attr.c | 13 +++++++++++++
2 files changed, 23 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/mips/umips-attr.c
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index bf69850..959a672 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -676,6 +676,9 @@ const char *mips_hi_relocs[NUM_SYMBOL_TYPES];
/* Target state for MIPS16. */
struct target_globals *mips16_globals;
+/* Target state for MICROMIPS. */
+struct target_globals *micromips_globals;
+
/* Cached value of can_issue_more. This is cached in mips_variable_issue hook
and returned from mips_sched_reorder2. */
static int cached_can_issue_more;
@@ -17182,6 +17185,13 @@ mips_set_compression_mode (unsigned int compression_mode)
else
restore_target_globals (mips16_globals);
}
+ else if (compression_mode & MASK_MICROMIPS)
+ {
+ if (!micromips_globals)
+ micromips_globals = save_target_globals_default_opts ();
+ else
+ restore_target_globals (micromips_globals);
+ }
else
restore_target_globals (&default_target_globals);
diff --git a/gcc/testsuite/gcc.target/mips/umips-attr.c b/gcc/testsuite/gcc.target/mips/umips-attr.c
new file mode 100644
index 0000000..f8c4517
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/umips-attr.c
@@ -0,0 +1,13 @@
+/* { dg-options "(-mmicromips)" } */
+
+int MICROMIPS
+foo (int a)
+{
+ return a;
+}
+
+int NOMICROMIPS
+foo2 (int a)
+{
+ return a;
+}
--
2.2.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH, MIPS]: Fix internal compiler error: in check_bool_attrs, at recog.c:2218 for micromips attribute
2015-05-14 14:51 [PATCH, MIPS]: Fix internal compiler error: in check_bool_attrs, at recog.c:2218 for micromips attribute Robert Suchanek
@ 2015-05-18 11:15 ` Matthew Fortune
2015-05-18 14:42 ` Robert Suchanek
0 siblings, 1 reply; 7+ messages in thread
From: Matthew Fortune @ 2015-05-18 11:15 UTC (permalink / raw)
To: Robert Suchanek, Catherine_Moore; +Cc: gcc-patches
> This patch fixes an internal compiler error when micromips/nomicromips
> attributes are used.
>
> The problem here was that the cached boolean attributes for the current
> target did not agree with the uncached attributes throwing an assertion
> error.
>
> It appears that saving and restoring the state for micromips was
> missing, just like there is for mips16. OK to apply?
Just to check, the reason we don't see this in the current testsuite is that
there is no test with both MIPS and microMIPS functions?
> gcc/
> * config/mips/mips.c (micromips_globals): New variable.
> (mips_set_compression_mode): Save and reinitialize target-dependent
> state for microMIPS.
>
> gcc/testsuite
> * gcc.target/mips/umips-attr.c: New test.
OK.
Matthew
> ---
> gcc/config/mips/mips.c | 10 ++++++++++
> gcc/testsuite/gcc.target/mips/umips-attr.c | 13 +++++++++++++
> 2 files changed, 23 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/mips/umips-attr.c
>
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index
> bf69850..959a672 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -676,6 +676,9 @@ const char *mips_hi_relocs[NUM_SYMBOL_TYPES];
> /* Target state for MIPS16. */
> struct target_globals *mips16_globals;
>
> +/* Target state for MICROMIPS. */
> +struct target_globals *micromips_globals;
> +
> /* Cached value of can_issue_more. This is cached in
> mips_variable_issue hook
> and returned from mips_sched_reorder2. */ static int
> cached_can_issue_more; @@ -17182,6 +17185,13 @@
> mips_set_compression_mode (unsigned int compression_mode)
> else
> restore_target_globals (mips16_globals);
> }
> + else if (compression_mode & MASK_MICROMIPS)
> + {
> + if (!micromips_globals)
> + micromips_globals = save_target_globals_default_opts ();
> + else
> + restore_target_globals (micromips_globals);
> + }
> else
> restore_target_globals (&default_target_globals);
>
> diff --git a/gcc/testsuite/gcc.target/mips/umips-attr.c
> b/gcc/testsuite/gcc.target/mips/umips-attr.c
> new file mode 100644
> index 0000000..f8c4517
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/mips/umips-attr.c
> @@ -0,0 +1,13 @@
> +/* { dg-options "(-mmicromips)" } */
> +
> +int MICROMIPS
> +foo (int a)
> +{
> + return a;
> +}
> +
> +int NOMICROMIPS
> +foo2 (int a)
> +{
> + return a;
> +}
> --
> 2.2.2
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH, MIPS]: Fix internal compiler error: in check_bool_attrs, at recog.c:2218 for micromips attribute
2015-05-18 11:15 ` Matthew Fortune
@ 2015-05-18 14:42 ` Robert Suchanek
2015-05-18 14:57 ` Maciej W. Rozycki
0 siblings, 1 reply; 7+ messages in thread
From: Robert Suchanek @ 2015-05-18 14:42 UTC (permalink / raw)
To: Matthew Fortune, Catherine_Moore; +Cc: gcc-patches
Hi Matthew,
> > This patch fixes an internal compiler error when micromips/nomicromips
> > attributes are used.
> >
> > The problem here was that the cached boolean attributes for the current
> > target did not agree with the uncached attributes throwing an assertion
> > error.
> >
> > It appears that saving and restoring the state for micromips was
> > missing, just like there is for mips16. OK to apply?
>
> Just to check, the reason we don't see this in the current testsuite is that
> there is no test with both MIPS and microMIPS functions?
Correct. The testsuite either tests microMIPS or MIPS but not both in the same TU.
> > gcc/
> > * config/mips/mips.c (micromips_globals): New variable.
> > (mips_set_compression_mode): Save and reinitialize target-dependent
> > state for microMIPS.
> >
> > gcc/testsuite
> > * gcc.target/mips/umips-attr.c: New test.
>
> OK.
Committed as r223294.
Regards,
Robert
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH, MIPS]: Fix internal compiler error: in check_bool_attrs, at recog.c:2218 for micromips attribute
2015-05-18 14:42 ` Robert Suchanek
@ 2015-05-18 14:57 ` Maciej W. Rozycki
2015-05-19 18:59 ` Robert Suchanek
0 siblings, 1 reply; 7+ messages in thread
From: Maciej W. Rozycki @ 2015-05-18 14:57 UTC (permalink / raw)
To: Robert Suchanek; +Cc: Matthew Fortune, Catherine_Moore, gcc-patches
On Mon, 18 May 2015, Robert Suchanek wrote:
> > Just to check, the reason we don't see this in the current testsuite is that
> > there is no test with both MIPS and microMIPS functions?
>
> Correct. The testsuite either tests microMIPS or MIPS but not both in
> the same TU.
We could add -mflip-micromips complementing -mflip-mips16 and use that
for testing too. Chances are it'd reveal further issues. Looking at how
-mflip-mips16 has been implemented it does not appear to me adding
-mflip-micromips would be a lot of effort.
Maciej
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH, MIPS]: Fix internal compiler error: in check_bool_attrs, at recog.c:2218 for micromips attribute
2015-05-18 14:57 ` Maciej W. Rozycki
@ 2015-05-19 18:59 ` Robert Suchanek
2015-05-20 10:17 ` Matthew Fortune
0 siblings, 1 reply; 7+ messages in thread
From: Robert Suchanek @ 2015-05-19 18:59 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: Matthew Fortune, Catherine_Moore, gcc-patches, Steve Ellcey
Hi,
The original patch had a missing declaration of micromips_globals in mips.h
that appears to be the cause of segmentation faults when building mips-mti-linux-gnu.
I didn't get any failures just before the submission neither on mips-img-linux-gnu
nor mips64el-linux-gnu and the test case is too trivial to trigger the ICE.
Below is the missing line. With this change mips-mti-linux-gnu builds fine.
The trunk is unstable and needed another patch from PR66181 to build
Glibc. Ok to commit?
> We could add -mflip-micromips complementing -mflip-mips16 and use that
> for testing too. Chances are it'd reveal further issues. Looking at how
> -mflip-mips16 has been implemented it does not appear to me adding
> -mflip-micromips would be a lot of effort.
I'm in favour of adding such a switch since the testsuite doesn't cover
a mixture of MIPS and microMIPS code.
Regards,
Robert
gcc/
* config/mips/mips.h (micromips_globals): Declare.
---
gcc/config/mips/mips.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 0ea4e6d..85c8a97 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -3108,6 +3108,7 @@ extern const struct mips_cpu_info *mips_arch_info;
extern const struct mips_cpu_info *mips_tune_info;
extern unsigned int mips_base_compression_flags;
extern GTY(()) struct target_globals *mips16_globals;
+extern GTY(()) struct target_globals *micromips_globals;
#endif
/* Enable querying of DFA units. */
--
2.2.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH, MIPS]: Fix internal compiler error: in check_bool_attrs, at recog.c:2218 for micromips attribute
2015-05-19 18:59 ` Robert Suchanek
@ 2015-05-20 10:17 ` Matthew Fortune
2015-05-20 10:42 ` Robert Suchanek
0 siblings, 1 reply; 7+ messages in thread
From: Matthew Fortune @ 2015-05-20 10:17 UTC (permalink / raw)
To: Robert Suchanek, Maciej W. Rozycki
Cc: Catherine_Moore, gcc-patches, Steve Ellcey
> > We could add -mflip-micromips complementing -mflip-mips16 and use
> > that for testing too. Chances are it'd reveal further issues.
> > Looking at how
> > -mflip-mips16 has been implemented it does not appear to me adding
> > -mflip-micromips would be a lot of effort.
>
> I'm in favour of adding such a switch since the testsuite doesn't cover
> a mixture of MIPS and microMIPS code.
It certainly seems that we need a bit more coverage here in order that
we can mostly stick to testing one or two MIPS configurations per commit.
We'll have some MIPS machines in the compile farm shortly which may allow
us to at least do the full all-config build of the toolchain more easily
even if that doesn't extend to testing all the configs.
>
> Regards,
> Robert
>
> gcc/
> * config/mips/mips.h (micromips_globals): Declare.
OK, thanks.
Matthew
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-05-20 10:38 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-14 14:51 [PATCH, MIPS]: Fix internal compiler error: in check_bool_attrs, at recog.c:2218 for micromips attribute Robert Suchanek
2015-05-18 11:15 ` Matthew Fortune
2015-05-18 14:42 ` Robert Suchanek
2015-05-18 14:57 ` Maciej W. Rozycki
2015-05-19 18:59 ` Robert Suchanek
2015-05-20 10:17 ` Matthew Fortune
2015-05-20 10:42 ` Robert Suchanek
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).