* [Patch, mips] Fix compiler abort with -mips32r2 -mips16 -msynci
@ 2012-07-17 21:05 Steve Ellcey
2012-07-18 17:30 ` Richard Sandiford
0 siblings, 1 reply; 5+ messages in thread
From: Steve Ellcey @ 2012-07-17 21:05 UTC (permalink / raw)
To: gcc-patches
While working on my favorite mips option (-msynci) I noticed an odd thing.
If I compile with '-mips32 -mips16 -msynci' I got a warning about synci not
being supported but if I compiled with '-mips32r2 -mips16 -msynci' I did not
get a warning, even though -mips16 mode does not support synci. Furthermore
if I compiled a program that called __builtin___clear_cache with '-mips32r2
-mips16 -msynci', the compiler would abort.
In mips.h we have:
/* ISA includes synci, jr.hb and jalr.hb. */
#define ISA_HAS_SYNCI ((ISA_MIPS32R2 \
|| ISA_MIPS64R2) \
&& !TARGET_MIPS16)
What I found was that in mips_option_override, where we check this macro
to generate the warning we have this code at the front of the function:
/* Process flags as though we were generating non-MIPS16 code. */
mips_base_mips16 = TARGET_MIPS16;
target_flags &= ~MASK_MIPS16;
Then later, we check ISA_HAS_SYNCI, but at that point TARGET_MIPS16 is
always false because of the above lines.
I looked at changing ISA_HAS_SYNCI to use target_flag_explicit but that
seems like the wrong thing to do for the use of ISA_HAS_SYNCI in mips.md.
Then I modified the if statement in mips_option_override but that resulted
in the use of 'mips32r2 -mips16 -msynci' giving an odd warning message:
warning: the âmips32r2â architecture does not support the synci instruction
But of course mips32r2 does support synci, it is -mips16 that does not. So I
added a new if statement with an explicit check against mips_base_mips16 to
give a better warning.
OK to checkin?
Steve Ellcey
sellcey@mips.com
2012-07-17 Steve Ellcey <sellcey@mips.com>
* config/mips/mips.c (mips_option_override): Fix check for -mips16
-msynci combination of flags.
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 7356ce5..889cfb5 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -16212,6 +16212,14 @@ mips_option_override (void)
target_flags &= ~MASK_SYNCI;
}
+ /* ISA_HAS_SYNCI checks TARGET_MIPS16 but that was turned off at the
+ beginning of this function so we need to check mips_base_mips16. */
+ if (TARGET_SYNCI && mips_base_mips16)
+ {
+ warning (0, "the \'mips16\' ASE does not support the synci instruction");
+ target_flags &= ~MASK_SYNCI;
+ }
+
/* Only optimize PIC indirect calls if they are actually required. */
if (!TARGET_USE_GOT || !TARGET_EXPLICIT_RELOCS)
target_flags &= ~MASK_RELAX_PIC_CALLS;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch, mips] Fix compiler abort with -mips32r2 -mips16 -msynci
2012-07-17 21:05 [Patch, mips] Fix compiler abort with -mips32r2 -mips16 -msynci Steve Ellcey
@ 2012-07-18 17:30 ` Richard Sandiford
2012-07-18 22:20 ` Steve Ellcey
2012-07-19 17:09 ` Steve Ellcey
0 siblings, 2 replies; 5+ messages in thread
From: Richard Sandiford @ 2012-07-18 17:30 UTC (permalink / raw)
To: Steve Ellcey ; +Cc: gcc-patches
"Steve Ellcey " <sellcey@mips.com> writes:
> While working on my favorite mips option (-msynci) I noticed an odd thing.
> If I compile with '-mips32 -mips16 -msynci' I got a warning about synci not
> being supported but if I compiled with '-mips32r2 -mips16 -msynci' I did not
> get a warning, even though -mips16 mode does not support synci. Furthermore
> if I compiled a program that called __builtin___clear_cache with '-mips32r2
> -mips16 -msynci', the compiler would abort.
The abort sounds like the bug here. It's deliberate that things like
-msynci, -mbranch-likely, etc., are OK with -mips16. On the one hand,
you could compile with -mips16 but have an __attribute__((nomips16))
function that could benefit from using SYNCI. On the other, you could
compile without -mips16 but have an __attribute__((mips16)) function
that needs to avoid SYNCI.
-mips16 really just sets the default ISA mode for functions that don't
specify one. That's why override_options hides mips16ness so early on,
like you say.
Richard
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch, mips] Fix compiler abort with -mips32r2 -mips16 -msynci
2012-07-18 17:30 ` Richard Sandiford
@ 2012-07-18 22:20 ` Steve Ellcey
2012-07-19 17:09 ` Steve Ellcey
1 sibling, 0 replies; 5+ messages in thread
From: Steve Ellcey @ 2012-07-18 22:20 UTC (permalink / raw)
To: Richard Sandiford; +Cc: gcc-patches
On Wed, 2012-07-18 at 18:30 +0100, Richard Sandiford wrote:
> The abort sounds like the bug here. It's deliberate that things like
> -msynci, -mbranch-likely, etc., are OK with -mips16. On the one hand,
> you could compile with -mips16 but have an __attribute__((nomips16))
> function that could benefit from using SYNCI. On the other, you could
> compile without -mips16 but have an __attribute__((mips16)) function
> that needs to avoid SYNCI.
OK, I think that makes sense.
> -mips16 really just sets the default ISA mode for functions that don't
> specify one. That's why override_options hides mips16ness so early on,
> like you say.
Ah, I didn't really understand why we were hiding the -mips16 setting,
now I do.
I will see if I can figure out why we abort. The clear_cache insn in
mips.md looks a bit odd to me, there is the part that is executed when
TARGET_SYNCI is true and then a part that is only executed if
mips_cache_flush_func is defined. It looks like if
mips_cache_flush_func is not defined then we do nothing and I was
wondering if that is correct or not? Should mips_cache_flush_func
being NULL be an error? I am not even sure if you can make it NULL
given that it is given a default value in mips.opt.
My test case is:
void f()
{
int size = 40;
char *memory = __builtin_alloca(size);
__builtin___clear_cache(memory, memory + size);
}
And the abort with -mips32r2 -mips16 -msynci is:
x.c: In function âfâ:
x.c:6:1: error: unrecognizable insn:
}
^
(jump_insn 22 21 38 2 (set (pc)
(if_then_else (eq (reg:SI 207)
(reg/f:SI 196 [ D.1415 ]))
(label_ref 33)
(pc))) x.c:5 -1
(nil)
-> 33)
x.c:6:1: internal compiler error: in extract_insn, at recog.c:2129
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
If I can't figure out what is going on I will file a bug report.
Steve Ellcey
sellcey@mips.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch, mips] Fix compiler abort with -mips32r2 -mips16 -msynci
2012-07-18 17:30 ` Richard Sandiford
2012-07-18 22:20 ` Steve Ellcey
@ 2012-07-19 17:09 ` Steve Ellcey
2012-07-19 18:09 ` Richard Sandiford
1 sibling, 1 reply; 5+ messages in thread
From: Steve Ellcey @ 2012-07-19 17:09 UTC (permalink / raw)
To: Richard Sandiford; +Cc: gcc-patches
On Wed, 2012-07-18 at 18:30 +0100, Richard Sandiford wrote:
> The abort sounds like the bug here. It's deliberate that things like
> -msynci, -mbranch-likely, etc., are OK with -mips16. On the one hand,
> you could compile with -mips16 but have an __attribute__((nomips16))
> function that could benefit from using SYNCI. On the other, you could
> compile without -mips16 but have an __attribute__((mips16)) function
> that needs to avoid SYNCI.
>
> -mips16 really just sets the default ISA mode for functions that don't
> specify one. That's why override_options hides mips16ness so early on,
> like you say.
>
> Richard
Here is a new patch. It clears the MASK_SYNCI bit when setting the
MASK_MIPS16 bit in mips_set_mips16_mode. I did not add any warnings
because I didn't think we wanted a warning every time we compiled a
function in MIPS16 mode when using -msynci.
How does this patch look?
Steve Ellcey
sellcey@mips.com
2012-07-19 Steve Ellcey <sellcey@mips.com>
* config/mips/mips.c (mips_set_mips16_mode): Clear SYNCI_MASK in
MIPS16 mode.
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 7356ce5..00360f7 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -15652,6 +15652,9 @@ mips_set_mips16_mode (int mips16_p)
/* Switch to MIPS16 mode. */
target_flags |= MASK_MIPS16;
+ /* Turn off SYNCI if it was on, MIPS16 doesn't support it. */
+ target_flags &= ~MASK_SYNCI;
+
/* Don't run the scheduler before reload, since it tends to
increase register pressure. */
flag_schedule_insns = 0;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch, mips] Fix compiler abort with -mips32r2 -mips16 -msynci
2012-07-19 17:09 ` Steve Ellcey
@ 2012-07-19 18:09 ` Richard Sandiford
0 siblings, 0 replies; 5+ messages in thread
From: Richard Sandiford @ 2012-07-19 18:09 UTC (permalink / raw)
To: Steve Ellcey; +Cc: gcc-patches
Steve Ellcey <sellcey@mips.com> writes:
> 2012-07-19 Steve Ellcey <sellcey@mips.com>
>
> * config/mips/mips.c (mips_set_mips16_mode): Clear SYNCI_MASK in
> MIPS16 mode.
OK, thanks.
Richard
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-07-19 18:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-17 21:05 [Patch, mips] Fix compiler abort with -mips32r2 -mips16 -msynci Steve Ellcey
2012-07-18 17:30 ` Richard Sandiford
2012-07-18 22:20 ` Steve Ellcey
2012-07-19 17:09 ` Steve Ellcey
2012-07-19 18:09 ` Richard Sandiford
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).