FW: [PATCH] [MIPS] microMIPS gcc support
Richard Sandiford
rdsandiford@googlemail.com
Wed Jan 23 20:05:00 GMT 2013
Hi Catherine,
Thanks for the update. Despite the length of this reply, there isn't
anything major.
Please don't change all NOMIPS16 to NOCOMPRESSION in the testsuite.
Most NOMIPSs are there because of the drastically reduced MIPS16
instruction set. I would have expected most of them to pass with
microMIPS.
Instead, please add NOCOMPRESSION as an alternative to NOMIPS16,
and only change NOMIPS16 to NOCOMPRESSION in tests that also fail for
microMIPS. I think that should be a separate patch and isn't really
a prerequisite for the core patch going in, although obviously it would
better if the testsuite results were clean. Please explain why each
NOMIPS16 to NOCOMPRESSION change is needed; even if it's obvious to you,
it probably won't be to me.
There need to be more test casees. We should at least test:
* LWM and SWM (see gcc.target/mips/save-restore-* for possible templates)
* MOVEP
* positive LWP and SWP tests (rather than just the volatile "not used" one)
* the new asm constraints
(Unlike the NOCOMPRESSION bits, these should be part of the core patch.)
As a general comment, I think it would be good to have:
/* The ISA compression flags that are currently in effect. */
#define TARGET_COMPRESSION (target_flags & (MASK_MIPS16 | MASK_MICROMIPS))
I've used this in some of the comments below.
"Moore, Catherine" <Catherine_Moore@mentor.com> writes:
> Index: config/mips/linux-unwind.h
> ===================================================================
> --- config/mips/linux-unwind.h (revision 195304)
> +++ config/mips/linux-unwind.h (working copy)
> @@ -52,6 +52,11 @@
> _Unwind_Ptr new_cfa, reg_offset;
> int i;
>
> + /* microMIPS frame; the kernel does not have microMIPS signal
> + frames. */
> + if ((_Unwind_Ptr) pc & 3)
> + return _URC_END_OF_STACK;
Should mention MIPS16 too. E.g.:
/* A MIPS16 or microMIPS frame. Signal frames always use the standard
ISA encoding. */
> @@ -15991,18 +15995,33 @@ Generate MIPS16 code on alternating functions. Th
> for regression testing of mixed MIPS16/non-MIPS16 code generation, and is
> not intended for ordinary use in compiling user code.
>
> +@item -minterlink-compressed
> +@item -mno-interlink-compressed
> +@opindex minterlink-compressed
> +@opindex mno-interlink-compressed
> +Require (do not require) that code using the standard (uncompressed) MIPS ISA
> +be link-compatible with MIPS16 and microMIPS code.
> +
> +For example, non-MIPS16 code cannot jump directly to MIPS16 code; it must
> +either use a call or an indirect jump. The same applies to non-microMIPS
> +jumps to microMIPS code. @option{-minterlink-compressed} therefore disables
> +direct jumps unless GCC knows that the target of the jump is not compressed.
I think I'm reediting a previous suggestion, sorry, but:
For example, code using the standard ISA encoding cannot jump directly
to MIPS16 or microMIPS code; it must either use a call or an indirect jump.
@option{-minterlink-compressed} therefore disables direct jumps unless GCC
knows that the target of the jump is not compressed.
> +@item -minterlink-uncompressed
> +@item -mno-interlink-uncompressed
> +@opindex minterlink-compressed
> +@opindex mno-interlink-compressed
> +Require (do not require) that code using microMIPS instructions be
> +link-compatible with the standard (uncompressed) MIPS ISA.
The distinction between this and -minterlink-compressed seems confusing
to me, and it wasn't handled consistently in the patch (see the sibcall
and gnu-user.h comments below). I think it would be better to fold the
functionality into -minterlink-compressed, with the documentation
changed to:
Require (do not require) that code using the standard (uncompressed) MIPS ISA
be link-compatible with MIPS16 and microMIPS code, and vice versa.
> Index: config/mips/gnu-user.h
> ===================================================================
> --- config/mips/gnu-user.h (revision 195351)
> +++ config/mips/gnu-user.h (working copy)
> @@ -137,3 +137,12 @@ extern const char *host_detect_local_cpu (int argc
> #define ENDFILE_SPEC \
> GNU_USER_TARGET_MATHFILE_SPEC " " \
> GNU_USER_TARGET_ENDFILE_SPEC
> +
> +#undef SUBTARGET_OVERRIDE_OPTIONS
> +#define SUBTARGET_OVERRIDE_OPTIONS \
> +do { \
> + /* microMIPS PLT entries are non-microMIPS. */ \
> + TARGET_INTERLINK_COMPRESSED = 1; \
> +} while (0)
> +
> +
Hmm, that sounds like a reason to set TARGET_INTERLINK_UNCOMPRESSED
rather than TARGET_INTERLINK_COMPRESSED. I.e. we need to avoid direct
branches from microMIPS code to standard PLTs.
But that means that microMIPS code can't even jump directly to functions
that have a micromips attribute when the call might go via a PLT.
TARGET_INTERLINK_(UN)COMPRESSED doesn't cover that case. I think instead
we need to handle it directly in mips_function_ok_for_sibcall,
keyed off TARGET_ABICALLS_PIC0. Specific suggestion below.
> +(define_insn "*lwp"
> + [(parallel [(set (match_operand:SI 0 "d_operand")
> + (match_operand:SI 1 "non_volatile_mem_operand"))
> + (set (match_operand:SI 2 "d_operand")
> + (match_operand:SI 3 "non_volatile_mem_operand"))])]
> +
> + "TARGET_MICROMIPS
> + && umips_load_store_pair_p (true, operands)"
> + {
> + umips_output_load_store_pair (true, operands);
> + return "";
> + }
Very minor, sorry, but other MIPS patterns put multiline { ... } blocks
in column 0. Same for *swp, but *movep<MOVEP1:mode><MOVEP2:mode>
is already OK.
> + [(set_attr "type" "load")
> + (set_attr "mode" "SI")
> + (set_attr "can_delay" "no")])
Please add a comment saying why LWP can't go in a delay slot.
Same for SWP and MOVEP.
> +(define_insn "mips_jraddiusp"
> + [(parallel [(return)
> + (use (reg:SI 31))
> + (set (reg:SI 29)
> + (plus:SI (reg:SI 29)
> + (match_operand 0 "const_int_operand")))])]
> + "TARGET_MICROMIPS"
> + "jraddiusp\t%0"
> + [(set_attr "type" "trap")
> + (set_attr "mode" "SI")
> + (set_attr "can_delay" "no")])
Is this pattern used? If not, please leave it out for now.
If it is used, please add a testcase.
> +; For movep
> +(define_peephole2
> + [(set (match_operand:MOVEP1 0 "register_operand" "")
> + (match_operand:MOVEP1 1 "movep_operand" ""))
> + (set (match_operand:MOVEP2 2 "register_operand" "")
> + (match_operand:MOVEP2 3 "movep_operand" ""))]
> + "TARGET_MICROMIPS
> + && umips_movep_target_p (operands[0], operands[2])"
> + [(parallel [(set (match_dup 0) (match_dup 1))
> + (set (match_dup 2) (match_dup 3))])]
> +)
Probably not worth having movep_operand, because all the checking
is done by umips_movep_target_p. reg_or_0_operand should be OK.
> Index: config/mips/constraints.md
> ===================================================================
> --- config/mips/constraints.md (revision 195351)
> +++ config/mips/constraints.md (working copy)
> @@ -232,6 +232,26 @@
> "@internal"
> (match_operand 0 "low_bitmask_operand"))
>
> +(define_memory_constraint "ZC"
> + "When compiling microMIPS code, this constraing matches a memory operand
> + whose address is formed from a base register and a 12-bit offset. These
> + operands can be used for microMIPS instructions such as @code{ll} and
> + @code{sc}. When not compiling for microMIPS code, @code{ZC} is
> + equivalent to @code{R}."
> + (and (match_code "mem")
> + (ior (and (match_test "TARGET_MICROMIPS")
> + (match_test "umips_12bit_offset_address_p (XEXP (op, 0), mode)"))
> + (match_test "mips_address_insns (XEXP (op, 0), mode, false)"))))
The last line should be:
(match_test "mips_address_insns (XEXP (op, 0), mode, false) == 1")
to match "R". The documentation makes it sound like an if-then-else,
but the implementation isn't. Should it be:
(and (match_code "mem")
(if_then_else
(match_test "TARGET_MICROMIPS")
(match_test "umips_12bit_offset_address_p (XEXP (op, 0), mode)"))
(match_test "mips_address_insns (XEXP (op, 0), mode, false) == 1"))))
instead?
> +(define_address_constraint "ZD"
> + "When compiling microMIPS code, this constraint matches an address operand
> + that is formed from a base register and a 12-bit offset. These operands
> + can be used for microMIPS instructions such as @code{prefetch}. When
> + not compiling for microMIPS code, @code{YC} is equivalent to @code{p}."
> + (ior (and (match_test "TARGET_MICROMIPS")
> + (match_test "umips_12bit_offset_address_p (op, mode)"))
> + (match_test "mips_address_insns (op, mode, false)")))
Same comments here.
> +;; Return 1 if the operand is in non-volatile memory. Note that during the
> +;; RTL generation phase, memory_operand does not return TRUE for volatile
> +;; memory references. So this function allows us to recognize volatile
> +;; references where it's safe.
> +(define_predicate "non_volatile_mem_operand"
> + (and (match_operand 0 "memory_operand")
> + (not (match_test "MEM_VOLATILE_P (op)"))))
I don't understand the comment. We're trying to avoid all volatile references.
> +minterlink-uncompressed
> +Target Report Var(TARGET_INTERLINK_UNCOMPRESSED) Init(1)
> +Generated code that is link-compatible with the standard (uncompressed)
> +MIPS ISA.
Do multi-line documentation strings work? Please check the --target-help
output to make sure.
> Index: config/mips/mips.c
> ===================================================================
> --- config/mips/mips.c (revision 195351)
> +++ config/mips/mips.c (working copy)
> @@ -77,6 +77,9 @@ along with GCC; see the file COPYING3. If not see
> preserve the maximum stack alignment. We therefore use a value
> of 0x7ff0 in this case.
>
> + microMIPS LWM and SWM support 12-bit offsets (from -0x800 to 0x7ff),
> + so we use a maximum of 0x7f0 for TARGET_MICROMIPS.
> +
> MIPS16e SAVE and RESTORE instructions can adjust the stack pointer by
> up to 0x7f8 bytes and can usually save or restore all the registers
> that we need to save or restore. (Note that we can only use these
> @@ -87,7 +90,8 @@ along with GCC; see the file COPYING3. If not see
> to save and restore registers, and to allocate and deallocate the top
> part of the frame. */
> #define MIPS_MAX_FIRST_STACK_STEP \
> - (!TARGET_MIPS16 ? 0x7ff0 \
> + ((!TARGET_MIPS16 && !TARGET_MICROMIPS) ? 0x7ff0 \
> + : TARGET_MICROMIPS ? 0x7f0 \
> : GENERATE_MIPS16E_SAVE_RESTORE ? 0x7f8 \
> : TARGET_64BIT ? 0x100 : 0x400)
I'd prefer:
(TARGET_MICROMIPS ? 0x7f0 \
: !TARGET_MIPS16 ? 0x7ff0 \
: GENERATE_MIPS16E_SAVE_RESTORE ? 0x7f8 \
: TARGET_64BIT ? 0x100 : 0x400)
The comment doesn't really explain the reason for 0x7f0 rather than 0x7f8.
Is this in anticipation of future n32 and n64 support, where the stack
must be 16-byte aligned? If so, that's OK, but worth mentioning.
> @@ -1167,10 +1174,11 @@ mflip_mips16_use_mips16_p (tree decl)
> const char *name;
> hashval_t hash;
> void **slot;
> + bool base_is_mips16 = mips_base_compression_flags & MASK_MIPS16;
Please make this:
bool base_is_mips16 = (mips_base_compression_flags & MASK_MIPS16) != 0;
> @@ -1257,12 +1252,57 @@ mips_use_debug_exception_return_p (tree type)
> TYPE_ATTRIBUTES (type)) != NULL;
> }
>
> -/* Return true if function DECL is a MIPS16 function. Return the ambient
> - setting if DECL is null. */
> +/* Return the set of compression modes that are explicitly required
> + by DECL. */
>
> -static bool
> -mips_use_mips16_mode_p (tree decl)
> +static unsigned int
> +mips_get_compress_on_flags (tree decl)
> {
> + unsigned int flags = 0;
> +
> + if (!decl || !DECL_P (decl))
> + return flags;
Is this check necessary? Same for mips_get_compress_off_flags.
> @@ -1297,29 +1337,47 @@ static void
> mips_insert_attributes (tree decl, tree *attributes)
> {
> const char *name;
> - bool mips16_p, nomips16_p;
> + unsigned int compression_p, nocompression_p;
No _p, since these aren't booleans now. _flags would be better.
> /* Check for "mips16" and "nomips16" attributes. */
> - mips16_p = lookup_attribute ("mips16", *attributes) != NULL;
> - nomips16_p = lookup_attribute ("nomips16", *attributes) != NULL;
> + compression_p = mips_get_compress_on_flags (decl);
> + nocompression_p = mips_get_compress_off_flags (decl);
> +
> if (TREE_CODE (decl) != FUNCTION_DECL)
> {
> - if (mips16_p)
> + if (compression_p & MASK_MIPS16)
> error ("%qs attribute only applies to functions", "mips16");
> - if (nomips16_p)
> + if (nocompression_p & MASK_MIPS16)
> error ("%qs attribute only applies to functions", "nomips16");
> +
> + if (nocompression_p & MASK_MICROMIPS
> + && nocompression_p & MASK_MIPS16)
> + error ("%qs attribute only applies to functions", "nocompression");
This case needs to come first, otherwise nomips16 will be reported instead.
Although...
> + else
> + {
> + if (compression_p & MASK_MICROMIPS)
> + error ("%qs attribute only applies to functions", "micromips");
> + if (nocompression_p & MASK_MICROMIPS)
> + error ("%qs attribute only applies to functions", "nomicromips");
> + }
...I think it would be easier to have:
/* Return the attribute name associated with MASK_MIPS16 and MASK_MICROMIPS
flags FLAGS. */
static const char *
mips_get_compress_on_name (unsigned int flags)
{
if (flags == MASK_MIPS16)
return "mips16";
return "micromips";
}
/* Return the attribute that forbids MASK_MIPS16 and MASK_MICROMIPS
flags FLAGS. */
static const char *
mips_get_compress_off_name (unsigned int flags)
{
if (flags == MASK_MIPS16)
return "nomips16";
if (flags == MASK_MICROMIPS)
return "nomicromips";
return "nocompression";
}
Then:
if (nocompression_flags)
error ("%qs attribute only applies to functions",
mips_get_compress_off_name (nocompression_flags));
if (compression_flags)
error ("%qs attribute only applies to functions",
mips_get_compress_on_name (compression_flags));
> }
> else
> {
> - mips16_p |= mips_mips16_decl_p (decl);
> - nomips16_p |= mips_nomips16_decl_p (decl);
> - if (mips16_p || nomips16_p)
> + if (compression_p & MASK_MIPS16
> + && nocompression_p & (MASK_MIPS16 | MASK_MICROMIPS))
> {
> + /* DECL cannot be simultaneously "mips16" and "nocompression". */
> + error ("%qE cannot have both %<mips16%> and "
> + "%<nocompression%> attributes",
> + DECL_NAME (decl));
> + }
> + else if (compression_p & MASK_MIPS16
> + && nocompression_p & MASK_MIPS16)
> + {
> /* DECL cannot be simultaneously "mips16" and "nomips16". */
> - if (mips16_p && nomips16_p)
> - error ("%qE cannot have both %<mips16%> and "
> - "%<nomips16%> attributes",
> - DECL_NAME (decl));
> + error ("%qE cannot have both %<mips16%> and "
> + "%<nomips16%> attributes",
> + DECL_NAME (decl));
> }
> else if (TARGET_FLIP_MIPS16 && !DECL_ARTIFICIAL (decl))
> {
> @@ -1329,6 +1387,39 @@ mips_insert_attributes (tree decl, tree *attribute
> name = mflip_mips16_use_mips16_p (decl) ? "mips16" : "nomips16";
> *attributes = tree_cons (get_identifier (name), NULL, *attributes);
> }
> +
> + if (compression_p & MASK_MICROMIPS
> + && nocompression_p & (MASK_MIPS16 | MASK_MICROMIPS))
> + {
> + /* DECL cannot be simultaneously "micromips" and "nocompression". */
> + error ("%qE cannot have both %<micromips%> and "
> + "%<nocompression%> attributes",
> + DECL_NAME (decl));
> + }
> + else if (compression_p & MASK_MICROMIPS
> + && nocompression_p & MASK_MICROMIPS)
> + {
> + /* DECL cannot be simultaneously "micromips" and "nomicromips". */
> + error ("%qs cannot have both %<micromips%> and "
> + "%<nomicromips%> attributes",
> + IDENTIFIER_POINTER (DECL_NAME (decl)));
> + }
All the above then becomes:
if (compression_flags && nocompression_flags)
error ("%qE cannot have both %qs and %qs attributes",
DECL_NAME (decl), mips_get_compress_on_name (compression_flags),
mips_get_compress_off_name (nocompression_flags));
> +
> + if (compression_p & MASK_MIPS16
> + && compression_p & MASK_MICROMIPS)
> + error ("%qs cannot have both %<mips16%> and %<micromips%> attributes",
> + IDENTIFIER_POINTER (DECL_NAME (decl)));
%qE and no IDENTIFIER_POINTER. Please use the %qs form here too,
with "mips16" and "micromips" as error() arguments, so that one
translation covers both this case and the previous one.
> + /* If DECL is "nocompression" set the "nomips16" and
> + "nomicromips" attributes. */
> + if (nocompression_p & MASK_MIPS16
> + && nocompression_p & MASK_MICROMIPS)
> + {
> + name = "nomips16";
> + *attributes = tree_cons (get_identifier (name), NULL, *attributes);
> + name = "nomicromips";
> + *attributes = tree_cons (get_identifier (name), NULL, *attributes);
> + }
Hmm, why is this necessary? Anything that cares should be using
mips_get_compress_off_flags.
> @@ -1338,12 +1429,9 @@ static tree
> mips_merge_decl_attributes (tree olddecl, tree newdecl)
> {
> /* The decls' "mips16" and "nomips16" attributes must match exactly. */
> - if (mips_mips16_decl_p (olddecl) != mips_mips16_decl_p (newdecl))
> + if (mips_get_compress_mode (olddecl) != mips_get_compress_mode (newdecl))
> error ("%qE redeclared with conflicting %qs attributes",
> - DECL_NAME (newdecl), "mips16");
> - if (mips_nomips16_decl_p (olddecl) != mips_nomips16_decl_p (newdecl))
> - error ("%qE redeclared with conflicting %qs attributes",
> - DECL_NAME (newdecl), "nomips16");
> + DECL_NAME (newdecl), "compression");
This should be:
unsigned int diff;
diff = (mips_get_compress_on_flags (olddecl)
^ mips_get_compress_on_flags (newdecl));
if (diff)
error ("%qE redeclared with conflicting %qs attributes",
DECL_NAME (newdecl), mips_get_compress_on_name (diff));
diff = (mips_get_compress_off_flags (olddecl)
^ mips_get_compress_off_flags (newdecl));
if (diff)
error ("%qE redeclared with conflicting %qs attributes",
DECL_NAME (newdecl), mips_get_compress_off_name (diff));
> @@ -2300,6 +2388,20 @@ mips_address_insns (rtx x, enum machine_mode mode,
> return 0;
> }
>
> +/* Return true if X is a legitimate address with a 12-bit offset.
> + MODE is the mode of the value being accessed. */
> +
> +bool
> +umips_12bit_offset_address_p (rtx x, enum machine_mode mode)
> +{
> + struct mips_address_info addr;
> +
> + return (mips_classify_address(&addr, x, mode, false)
Space before '('
> + && addr.type == ADDRESS_REG
> + && CONST_INT_P (addr.offset)
> + && UMIPS_12BIT_OFFSET_P (addr.offset));
Isn't there a missing INTVAL here? Please check the patch to make sure
there are no compiler warnings.
> @@ -6908,6 +7017,8 @@ mips_split_call (rtx insn, rtx call_pattern)
> static bool
> mips_function_ok_for_sibcall (tree decl, tree exp ATTRIBUTE_UNUSED)
> {
> + unsigned int compression_mode = mips_get_compress_mode (decl);
> +
Might as well move this down to where it's needed, especially now that
we're C++.
> if (!TARGET_SIBCALLS)
> return false;
>
> @@ -6916,22 +7027,45 @@ mips_function_ok_for_sibcall (tree decl, tree exp
> if (mips_interrupt_type_p (TREE_TYPE (current_function_decl)))
> return false;
>
> + if (TARGET_MICROMIPS)
> + {
> + /* We can't do a sibcall if the called function is a MIPS32 function. */
s/is a MIPS32 function/isn't microMIPS/, to avoid confusion with
the MIPS32 ISA.
> + if (decl
> + && (compression_mode & MASK_MICROMIPS) == 0
> + && const_call_insn_operand (XEXP (DECL_RTL (decl), 0), VOIDmode))
> + return false;
> +
> + /* When -minterlink-compressed is in effect, assume that non-locally-binding
> + functions could be MIPS32 ones unless an attribute explicitly tells
> + us otherwise. */
> + if (TARGET_INTERLINK_COMPRESSED
> + && decl
> + && (DECL_EXTERNAL (decl) || !targetm.binds_local_p (decl))
> + && (compression_mode & MASK_MICROMIPS) == 0
> + && const_call_insn_operand (XEXP (DECL_RTL (decl), 0), VOIDmode))
> + return false;
We're dealing here with branches from microMIPS to non-microMIPS,
so shouldn't this be TARGET_INTERLINK_UNCOMPRESSED?
We need to use mips_get_compress_on_flags rather than mips_get_compress_mode
for the second condition, because we can only assume that external functions
are micromips if they have been explicitly tagged.
> +
> + /* Otherwise OK. */
> + return true;
> + }
> +
There's too much cut-&-paste of the MIPS16 code here, so that it
becomes difficult to see what the logic actually is. With the
suggested change to fold -minterlink-nocompressed into
-minterlink-compressed, the block above and:
> /* We can't do a sibcall if the called function is a MIPS16 function
> because there is no direct "jx" instruction equivalent to "jalx" to
> switch the ISA mode. We only care about cases where the sibling
> and normal calls would both be direct. */
> if (decl
> - && mips_use_mips16_mode_p (decl)
> + && ((compression_mode & (MASK_MIPS16 | MASK_MICROMIPS)) != 0)
> && const_call_insn_operand (XEXP (DECL_RTL (decl), 0), VOIDmode))
> return false;
>
> - /* When -minterlink-mips16 is in effect, assume that non-locally-binding
> - functions could be MIPS16 ones unless an attribute explicitly tells
> + /* When -minterlink-compressed is in effect, assume that non-locally-binding
> + functions could be MIPS16 or microMIPS unless an attribute explicitly tells
> us otherwise. */
> - if (TARGET_INTERLINK_MIPS16
> + if (TARGET_INTERLINK_COMPRESSED
> && decl
> && (DECL_EXTERNAL (decl) || !targetm.binds_local_p (decl))
> - && !mips_nomips16_decl_p (decl)
> + && (compression_mode & MASK_MICROMIPS) == 0
> + && (compression_mode & MASK_MIPS16) == 0
> && const_call_insn_operand (XEXP (DECL_RTL (decl), 0), VOIDmode))
> return false;
should be something like:
/* Direct Js are only possible to functions that use the same ISA encoding.
There is no JX counterpart of JALX. */
if (decl
&& const_call_insn_operand (XEXP (DECL_RTL (decl), 0), VOIDmode)
&& mips_call_may_need_jalx_p (decl))
return false;
with:
/* Return true if a call to DECL may need to use JALX. */
static bool
mips_call_may_need_jalx_p (decl)
{
/* If the current translation unit would use a different mode for DECL,
assume that the call needs JALX. */
if (mips_get_compress_mode (decl) != TARGET_COMPRESSION)
return true;
/* mips_get_compress_mode is always accurate for locally-binding
functions in the current translation unit. */
if (!DECL_EXTERNAL (decl) && targetm.binds_local_p (decl))
return false;
/* PLTs use the standard encoding, so calls that might go via PLTs
must use JALX. */
if (TARGET_COMPRESSED
&& TARGET_ABICALLS_PIC0
&& DECL_EXTERNAL (decl)
&& !targetm.binds_local_p (decl))
return true;
/* When -minterlink-compressed is in effect, assume that functions
could use a different encoding mode unless an attribute explicitly
tells us otherwise. */
if (TARGET_INTERLINK_COMPRESSED)
{
if (!TARGET_COMPRESSION && mips_get_compress_off_flags (decl) == 0)
return true;
if (TARGET_COMPRESSION && mips_get_compress_on_flags (decl) == 0)
return true;
}
return false;
}
The TARGET_ABICALLS_PIC0 case should deal with the gnu-user.h comment above.
> @@ -7881,6 +8018,20 @@ mips_print_operand_punctuation (FILE *file, int ch
> fputs (reg_names[STACK_POINTER_REGNUM], file);
> break;
>
> + case ':':
> + /* When reorder or noreorder with final_squence 0, the delay slot will
> + be a nop, so we just use the compact version for microMIPS. */
> + if (final_sequence == 0)
> + putc ('c', file);
> + break;
> +
> + case '!':
> + /* When reorder or noreorder with final_squence 0, the delay slot will
> + be a nop, so we just use the compact version for microMIPS. */
> + if (final_sequence == 0)
> + putc ('s', file);
> + break;
> +
Pasto: the second "the compact version" should be "16-bit delay slots".
> @@ -10198,7 +10349,7 @@ typedef void (*mips_save_restore_fn) (rtx, rtx);
> stack pointer. */
>
> static void
> -mips_save_restore_reg (enum machine_mode mode, int regno,
> +mips_save_restore_single_reg (enum machine_mode mode, int regno,
> HOST_WIDE_INT offset, mips_save_restore_fn fn)
> {
> rtx mem;
Please reindent the second line, although I'd prefer not to have this change;
more below.
> @@ -10208,6 +10359,23 @@ static void
> fn (gen_rtx_REG (mode, regno), mem);
> }
>
> +static void
> +mips_save_restore_registers (int start_reg, int end_reg, HOST_WIDE_INT *offset,
> + mips_save_restore_fn fn, HOST_WIDE_INT sp_offset)
> +{
> + int regno;
> +
> + for (regno = start_reg; regno > end_reg; regno--)
> + if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST))
> + {
> + /* Record the ra offset for use by mips_function_profiler. */
> + if (regno == RETURN_ADDR_REGNUM)
> + cfun->machine->frame.ra_fp_offset = *offset + sp_offset;
> + mips_save_restore_single_reg (word_mode, regno, *offset, fn);
> + *offset -= UNITS_PER_WORD;
> + }
> +}
> +
Function lacks a comment, although I'm not sure it's needed; see below.
> +static const unsigned int type[19] = { 0x00010000, 0x00030000, 0x00070000,
> + 0x000f0000, 0x001f0000, 0x003f0000,
> + 0x007f0000, 0x00ff0000, 0x40ff0000,
> + 0x80000000, 0x80010000, 0x80030000,
> + 0x80070000, 0x800f0000, 0x801f0000,
> + 0x803f0000, 0x807f0000, 0x80ff0000,
> + 0xc0ff0000 };
> +
> +static const unsigned int encode[19] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 16, 17,
> + 18, 19, 20, 21, 22, 23, 24, 25};
Please give these longer names, since they have file scope. E.g.:
/* The masks of GPRs that can be saved by a microMIPS SWM and restored
by an LWM. */
static const unsigned int umips_swm_mask[19] = {
0x00010000, 0x00030000, 0x00070000, 0x000f0000,
0x001f0000, 0x003f0000, 0x007f0000, 0x00ff0000, 0x40ff0000,
0x80000000, 0x80010000, 0x80030000, 0x80070000, 0x800f0000,
0x801f0000, 0x803f0000, 0x807f0000, 0x80ff0000, 0xc0ff0000
};
/* umips_swm_encoding[I] is the instruction encoding used to represent mask
umips_swm_mask[I]. */
static const unsigned int umips_swm_encoding[19] = {
1, 2, 3, 4,
5, 6, 7, 8, 9,
16, 17, 18, 19, 20,
21, 22, 23, 24, 25
};
> +/* Build microMIPS save or restore. FN is save or restore function.
> + OFFSET is the current stack offset.
> + Return true if we succeed creating save or restore. */
> +
> +static bool
> +umips_build_save_restore (mips_save_restore_fn fn,
> + HOST_WIDE_INT offset, HOST_WIDE_INT sp_offset)
The comment doesn't say what SP_OFFSET is, although with the comments below
I think the prototype should be:
/* Try to use a microMIPS LWM or SWM instruction to save or restore
as many GPRs in *MASK as possible. *OFFSET is the offset from the
stack pointer of the topmost save slot.
Remove from *MASK all registers that were handled using LWM and SWM.
Update *OFFSET so that it points to the first unused save slot. */
static void
umips_build_save_restore (mips_save_restore_fn fn, unsigned *mask
HOST_WIDE_INT *offset)
> +{
> + int i, num_of_reg;
> + unsigned int j;
> + rtx pattern, set, reg, mem;
> + HOST_WIDE_INT this_offset;
> + rtx this_base;
> +
> + /* LWM/SWM can only support offsets from -2048 to 2047. */
> + if (!UMIPS_12BIT_OFFSET_P (offset))
> + return false;
At this point OFFSET is still the offset of the top of the save area,
whereas the instruction operand is the address of the first saved register.
I think you need to check this later, after:
offset -= (UNITS_PER_WORD * (num_of_reg - 1));
> + if (i < 8 && (cfun->machine->frame.mask & 0xff000000))
> + mips_save_restore_registers (GP_REG_LAST, GP_REG_LAST - 8,
> + &offset, fn, sp_offset);
Any reason for handling this differently from the MIPS16 code? I prefer
passing back a mask of the unhandled registers, like it does. It would
make this function a bit simpler and would also allow the function to
be used in future with masks that don't exactly match. E.g. if we need
to save and restore registers $16-$20 + $23, we could use LWM for $16-$20
and LW for $23. The prototype suggested above was for this.
> + /* Adjust offset for output. */
> + num_of_reg = (encode[i] & 0xf) + (encode[i] >> 4);
> + offset -= (UNITS_PER_WORD * (num_of_reg - 1));
Very minor, sorry, but s/num_of_reg/nregs/;
> + /* Create the final PARALLEL. */
> + pattern = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (num_of_reg));
> +
> + this_base = stack_pointer_rtx;
> + this_offset = offset;
> +
> + /* For $16-$23, $30. */
> + for (j = 0; j < (encode[i] & 0xf); j++)
> + {
> + unsigned int regno;
> + long long offset = this_offset + j * UNITS_PER_WORD;
s/long long/HOST_WIDE_INT/.
> + pattern = emit_insn (pattern);
> + if (fn == mips_save_reg)
> + RTX_FRAME_RELATED_P(pattern) = 1;
Missing space before "(".
> @@ -10253,16 +10555,13 @@ mips_for_each_saved_gpr_and_fpr (HOST_WIDE_INT sp_
> need a nop in the epilogue if at least one register is reloaded in
> addition to return address. */
> offset = cfun->machine->frame.gp_sp_offset - sp_offset;
> - for (regno = GP_REG_LAST; regno >= GP_REG_FIRST; regno--)
> - if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST))
> - {
> - /* Record the ra offset for use by mips_function_profiler. */
> - if (regno == RETURN_ADDR_REGNUM)
> - cfun->machine->frame.ra_fp_offset = offset + sp_offset;
> - mips_save_restore_reg (word_mode, regno, offset, fn);
> - offset -= UNITS_PER_WORD;
> - }
>
> + if (!TARGET_MICROMIPS
> + || (TARGET_MICROMIPS
> + && !umips_build_save_restore (fn, offset, sp_offset)))
> + mips_save_restore_registers (GP_REG_LAST, GP_REG_FIRST,
> + &offset, fn, sp_offset);
> +
With the pass-back suggestion above, I think this should be:
offset = cfun->machine->frame.gp_sp_offset - sp_offset;
mask = cfun->machine->frame.mask
if (TARET_MICROMIPS16)
umips_build_save_restore (fn, &mask, &offset);
for (regno = GP_REG_LAST; regno >= GP_REG_FIRST; regno--)
if (BITSET_P (mask, regno - GP_REG_FIRST))
{
/* Record the ra offset for use by mips_function_profiler. */
if (regno == RETURN_ADDR_REGNUM)
cfun->machine->frame.ra_fp_offset = offset + sp_offset;
mips_save_restore_reg (word_mode, regno, offset, fn);
offset -= UNITS_PER_WORD;
}
> @@ -11078,6 +11357,7 @@ mips_deallocate_stack (rtx base, rtx offset, HOST_
> return;
>
> mips_frame_barrier ();
> +
> if (offset == const0_rtx)
> {
> emit_move_insn (stack_pointer_rtx, base);
> @@ -11196,9 +11476,9 @@ mips_expand_epilogue (bool sibcall_p)
> if (BITSET_P (mask, regno - GP_REG_FIRST))
> {
> offset -= UNITS_PER_WORD;
> - mips_save_restore_reg (word_mode, regno, offset, mips_restore_reg);
> + mips_save_restore_single_reg (word_mode, regno,
> + offset, mips_restore_reg);
> }
> -
> /* Restore the remaining registers and deallocate the final bit
> of the frame. */
> mips_frame_barrier ();
> @@ -11211,7 +11491,6 @@ mips_expand_epilogue (bool sibcall_p)
> mips_for_each_saved_acc (frame->total_size - step2, mips_restore_reg);
> mips_for_each_saved_gpr_and_fpr (frame->total_size - step2,
> mips_restore_reg);
> -
> if (cfun->machine->interrupt_handler_p)
> {
> HOST_WIDE_INT offset;
Please leave out these whitespace changes. I'd also prefer not to add
the _single_reg suffix to mips_save_restore_reg.
> @@ -11254,7 +11533,6 @@ mips_expand_epilogue (bool sibcall_p)
> /* Deallocate the final bit of the frame. */
> mips_deallocate_stack (stack_pointer_rtx, GEN_INT (step2), 0);
> }
> - gcc_assert (!mips_epilogue.cfa_restores);
>
> /* Add in the __builtin_eh_return stack adjustment. We need to
Please don't remove the assert. AIUI this was for epilogues that use
JRADDIUSP, which isn't part of the initial patch.
> @@ -16211,17 +16489,19 @@ mips_output_mi_thunk (FILE *file, tree thunk_fndec
> reload_completed = 0;
> }
>
> -/* The last argument passed to mips_set_mips16_mode, or negative if the
> - function hasn't been called yet. */
> +/* The last argument passed to mips_set_mips16_micromips_mode,
> + or negative if the function hasn't been called yet. */
> static int was_mips16_p = -1;
> +static int was_micromips_p = -1;
>
> /* Set up the target-dependent global state so that it matches the
> current function's ISA mode. */
>
> static void
> -mips_set_mips16_mode (int mips16_p)
> +mips_set_mips16_micromips_mode (int mips16_p, int micromips_p)
> {
> - if (mips16_p == was_mips16_p)
> + if (mips16_p == was_mips16_p
> + && micromips_p == was_micromips_p)
> return;
>
> /* Restore base settings of various flags. */
Now that we represent the mode using masks of MASK_MIPS16 and MASK_MICROMIPS,
I think it would be better to have a mask rather than two booleans here too.
I.e.:
/* The last argument passed to mips_set_mips16_micromips_mode,
or negative if the function hasn't been called yet. */
static unsigned int old_compression_mode = -1;
/* Set up the target-dependent global state for ISA mode COMPRESSION_MODE,
which is either MASK_MIPS16 or MASK_MICROMIPS. */
static void
mips_set_compression_mode (unsigned int compression_mode)
> @@ -16290,6 +16570,18 @@ static void
> /* Switch to normal (non-MIPS16) mode. */
> target_flags &= ~MASK_MIPS16;
>
> + if (micromips_p)
> + {
> + /* Switch to microMIPS mode. */
> + target_flags |= MASK_MICROMIPS;
> +
> + /* Avoid branch likely. */
> + target_flags &= ~MASK_BRANCHLIKELY;
> + }
> + else
> + /* Switch to normal (non-microMIPS) mode. */
> + target_flags &= ~MASK_MICROMIPS;
> +
With the change above, I think we should have:
target_flags &= ~(MASK_MIPS16 | MASK_MICROMIPS);
target_flags |= compression_mode;
outside of the "if" statement, with the code above becoming:
/* There are no microMIPS branch-likely instructions. */
if (TARGET_MICROMIPS)
target_flags &= ~MASK_BRANCHLIKELY;
> + /* Save the base compression state and process flags as though we
> + are generating uncompressed code. */
> + if (TARGET_MIPS16)
> + mips_base_compression_flags |= MASK_MIPS16;
> + if (TARGET_MICROMIPS)
> + mips_base_compression_flags |= MASK_MICROMIPS;
> +
> + target_flags &= ~MASK_MIPS16;
> + target_flags &= ~MASK_MICROMIPS;
Would prefer:
mips_base_compression_flags = TARGET_COMPRESSION;
target_flags &= ~TARGET_COMPRESSION;
> @@ -17088,6 +17397,316 @@ mips_mulsidi3_gen_fn (enum rtx_code ext_code)
> return signed_p ? gen_mulsidi3_32bit : gen_umulsidi3_32bit;
> }
> }
> +
> +
> +/* Return true if PATTERN matches the kind of instruction generated by
> + micromips_build_save_restore. SAVE_P is true for store. */
umips_build_save_restore
> +
> +bool
> +umips_save_restore_pattern_p (bool save_p, rtx pattern)
> +{
> + int n;
> + HOST_WIDE_INT first_offset = 0;
> + rtx first_base = 0;
> + unsigned int first_regno = 0;
> +
> + for (n = 0; n < XVECLEN (pattern, 0); n++)
> + {
> + rtx set, reg, mem, this_base;
> + HOST_WIDE_INT this_offset;
> + unsigned int this_regno;
> +
> + /* Check that we have a SET. */
> + set = XVECEXP (pattern, 0, n);
> + if (GET_CODE (set) != SET)
> + return false;
> +
> + /* Check that the SET is a load (if restoring) or a store
> + (if saving). */
> + mem = save_p ? SET_DEST (set) : SET_SRC (set);
> + if (!MEM_P (mem) || MEM_VOLATILE_P (mem))
> + return false;
> +
> + /* Check that the address is the sum of base and a possibly-zero
> + constant offset. Determine if the offset is in range. */
> + mips_split_plus (XEXP (mem, 0), &this_base, &this_offset);
> + if (!REG_P (this_base)
> + && UMIPS_12BIT_OFFSET_P (this_offset))
> + return false;
> +
> + if (n == 0)
> + {
> + first_base = this_base;
> + first_offset = this_offset;
> + }
The UMIPS_12BIT_OFFSET_P check should be in the "n == 0" arm.
> + else
> + {
> + /* Check if this_base is the same as first_base. */
> + if (REGNO (this_base) != REGNO (first_base))
> + return false;
> +
> + /* Check if this_offset is first_offset + UNITS_PER_WORD * n. */
> + if (this_offset != first_offset + UNITS_PER_WORD * n)
> + return false;
/* Check that the save slots are consecutive. */
if (REGNO (this_base) != REGNO (first_base)
|| this_offset != first_offset + UNITS_PER_WORD * n)
> + /* Make sure the order of regno is "$16-$23, $30, $31", "$16-$23, $30",
> + or "$31". */
> + this_regno = REGNO (reg);
> + if (n == 0)
> + {
> + if (this_regno != 16 && this_regno != 31)
> + return false;
> + first_regno = this_regno;
> + }
> + else if (n == 8) /* For s8. */
> + {
> + if (n == XVECLEN (pattern, 0) - 1)
> + {
> + if (this_regno != 30 && this_regno != 31)
> + return false;
> + }
> + else
> + {
> + if (this_regno != 30)
> + return false;
> + }
> + }
> + else if (n != XVECLEN (pattern, 0) - 1)
> + {
> + if (this_regno != first_regno + n)
> + return false;
> + }
> + else /* The last item. */
> + {
> + if ((this_regno != first_regno + n) && this_regno != 31)
> + return false;
> + }
> + }
Please instead check that the registers are in ascending order,
build a mask, and then check that mask against umips_swm_mask at the end.
> +}
> +/* Return true if MEM1 and MEM2 use the same base register, and the
Missing space between functions.
> + offset of MEM2 equals the offset of MEM1 plus 4. FIRST_REG is the
> + register into (from) which the contents of MEM1 will be loaded
> + (stored), depending on the value of LOAD_P.
> + SWAP_P is true when the 1st and 2nd instructions are swapped. */
> +
> +static bool
> +umips_load_store_pair_p_1 (bool load_p, bool swap_p, rtx first_reg,
> + rtx mem1, rtx mem2)
> +{
> + rtx base1, base2;
> + HOST_WIDE_INT offset1, offset2;
> +
> + if (!MEM_P (mem1) || !MEM_P (mem2))
> + return false;
> +
> + /* Make sure memory is base plus offset. */
> + if (GET_CODE (XEXP (mem1, 0)) != PLUS
> + || GET_CODE (XEXP (mem2, 0)) != PLUS
> + || GET_CODE (XEXP (XEXP (mem1, 0), 1)) != CONST_INT
> + || GET_CODE (XEXP (XEXP (mem2, 0), 1)) != CONST_INT)
> + return false;
> +
> + mips_split_plus (XEXP (mem1, 0), &base1, &offset1);
> + mips_split_plus (XEXP (mem2, 0), &base2, &offset2);
Please remove the:
/* Make sure memory is base plus offset. */
if (GET_CODE (XEXP (mem1, 0)) != PLUS
|| GET_CODE (XEXP (mem2, 0)) != PLUS
|| GET_CODE (XEXP (XEXP (mem1, 0), 1)) != CONST_INT
|| GET_CODE (XEXP (XEXP (mem2, 0), 1)) != CONST_INT)
return false;
part. The point of using mips_split_plus is that we want to handle
(reg) paired with (plus (reg) (const_int 4)) and (plus (reg) (const_int -4))
paired with (reg).
> + if (!REG_P (base1) || !rtx_equal_p (base1, base2))
> + return false;
> +
> + /* Avoid invalid load pair instructions. */
> + if (load_p && REGNO (first_reg) == REGNO (base1))
> + return false;
> +
> + /* We must avoid this case for anti-dependence.
> + Ex: lw $3, 4($3)
> + lw $2, 0($3)
> + first_reg is $2, but the base is $3. */
> + if (load_p && swap_p && (REGNO (first_reg) + 1) == REGNO (base1))
> + return false;
Redundant brackets around "REGNO (first_reg) + 1".
> +
> + if (swap_p
> + && offset2 + 4 != offset1)
> + return false;
> +
> + if (offset1 + 4 != offset2)
> + return false;
This always returns false for swap_p. Please add tests for both cases
to make sure that the code is triggering. E.g. you could have an inline
asm that takes two asm register variables, one in $2 and one in $3.
> +bool
> +umips_load_store_pair_p (bool load_p, rtx *operands)
Function needs a comment.
> +{
> + rtx reg1, reg2, mem1, mem2;
> +
> + if (load_p)
> + {
> + reg1 = operands[0];
> + reg2 = operands[2];
> + mem1 = operands[1];
> + mem2 = operands[3];
> + }
> + else
> + {
> + reg1 = operands[1];
> + reg2 = operands[3];
> + mem1 = operands[0];
> + mem2 = operands[2];
> + }
> +
> + if (REGNO (reg2) == REGNO (reg1) + 1)
> + return umips_load_store_pair_p_1 (load_p, false, reg1, mem1, mem2);
> +
> + else if (REGNO (reg1) == REGNO (reg2) + 1)
> + return umips_load_store_pair_p_1 (load_p, true, reg2, mem1, mem2);
> +
> + return false;
Excess indentation. Probably better without the "else", since this is
really early-exit style.
> +/* Return the assembly instruction for microMIPS lwp or swp.
> + LOAD_P is true for load. */
> +
> +static void
> +umips_output_load_store_pair_1 (bool load_p, rtx reg, rtx mem)
> +{
> + HOST_WIDE_INT offset;
> + rtx base, local_ops[3];
> +
> + gcc_assert (REG_P (reg) && MEM_P (mem));
> +
> + mips_split_plus (XEXP (mem, 0), &base, &offset);
> + gcc_assert (REG_P (base));
> +
> + local_ops[0] = reg;
> + local_ops[1] = gen_rtx_CONST_INT (Pmode, offset);
> + local_ops[2] = base;
> + if (load_p)
> + output_asm_insn ("lwp %0,%1(%2)", local_ops);
> + else
> + output_asm_insn ("swp %0,%1(%2)", local_ops);
> +}
This ought to be equivalent to:
rtx ops[] = { reg, mem };
if (load_p)
output_asm_insn ("lwp %0,%1", ops);
else
output_asm_insn ("swp %0,%1", ops);
> +/* Return true if reg1 and reg2 can be target of movep. */
> +
> +bool
> +umips_movep_target_p (rtx reg1, rtx reg2)
> +{
> + int regno1, regno2, pair;
> + unsigned int i;
> + static const int match[8] = {0x00000060, /* 5, 6 */
> + 0x000000a0, /* 5, 7 */
> + 0x000000c0, /* 6, 7 */
> + 0x00200010, /* 4, 21 */
> + 0x00400010, /* 4, 22 */
> + 0x00000030, /* 4, 5 */
> + 0x00000050, /* 4, 6 */
> + 0x00000090}; /* 4, 7 */
Should be formatted as:
static const int match[8] = {
0x00000060, /* 5, 6 */
...
};
> + for (i = 0; i < ARRAY_SIZE (match); i++)
> + {
> + if (pair == match[i])
> + return true;
> + }
Redundant { ... }.
> @@ -2452,18 +2459,48 @@ typedef struct mips_args {
> all calls should use assembly macros. Otherwise, all indirect
> calls should use "jr" or "jalr"; we will arrange to restore $gp
> afterwards if necessary. Finally, we can only generate direct
> - calls for -mabicalls by temporarily switching to non-PIC mode. */
> + calls for -mabicalls by temporarily switching to non-PIC mode.
> +
> + For microMIPS jal(r), we try to generate jal(r)s when a 16-bit
> + instruction is in the delay slot of jal(r). */
> #define MIPS_CALL(INSN, OPERANDS, TARGET_OPNO, SIZE_OPNO) \
> + (TARGET_MICROMIPS \
> + ? (TARGET_USE_GOT && !TARGET_EXPLICIT_RELOCS \
> + ? (TARGET_INTERLINK_UNCOMPRESSED \
> + ? "%*" INSN "%!\t%" #TARGET_OPNO "%/" \
> + : "%*" INSN "\t%" #TARGET_OPNO "%/") \
Is this special case really necessary? Normally when using assembly
macros we should leave the choice up to the assembler. On the one
hand we gain by having knowledge that the target is microMIPS,
while on the other we lose because the assembler can't fill the
delay slot with a 32-bit instruction from a preceding assembly macro.
Hopefully no-one really uses -mno-explicit-relocs these days anyway,
so I think this case should be handled in the same way for all modes.
I.e.:
TARGET_USE_GOT && !TARGET_EXPLICIT_RELOCS \
? "%*" INSN "\t%" #TARGET_OPNO "%/" \
: REG_P (OPERANDS[TARGET_OPNO]) \
? (mips_get_pic_call_symbol (OPERANDS, SIZE_OPNO) \
? ("%*.reloc\t1f,R_MIPS_JALR,%" #SIZE_OPNO "\n" \
"1:\t" INSN "r\t%" #TARGET_OPNO "%/") \
: TARGET_MICROMIPS && TARGET_INTERLINK_UNCOMPRESSED \
? "%*" INSN "r%!\t%" #TARGET_OPNO "%/" \
: "%*" INSN "r\t%" #TARGET_OPNO "%/") \
: MIPS_ABSOLUTE_JUMP ("%*" INSN "\t%" #TARGET_OPNO "%/")))
if that works.
> +
> +/* Similar to MIPS_CALL, but this is for MICROMIPS "j" to generate
> + "jrc" when nop is in the delay slot of "jr". */
> +#define MICROMIPS_J(OPERANDS, OPNO) \
> (TARGET_USE_GOT && !TARGET_EXPLICIT_RELOCS \
> - ? "%*" INSN "\t%" #TARGET_OPNO "%/" \
> - : (REG_P (OPERANDS[TARGET_OPNO]) \
> - && mips_get_pic_call_symbol (OPERANDS, SIZE_OPNO)) \
> - ? ("%*.reloc\t1f,R_MIPS_JALR,%" #SIZE_OPNO "\n" \
> - "1:\t" INSN "r\t%" #TARGET_OPNO "%/") \
> - : REG_P (OPERANDS[TARGET_OPNO]) \
> - ? "%*" INSN "r\t%" #TARGET_OPNO "%/" \
> - : MIPS_ABSOLUTE_JUMP ("%*" INSN "\t%" #TARGET_OPNO "%/"))
> -
> + ? "%*j\t%" #OPNO "%/" \
> + : REG_P (OPERANDS[OPNO]) \
> + ? "%*jr%:\t%" #OPNO \
> + : TARGET_ABICALLS && flag_pic \
> + ? (".option\tpic0\n\t" \
> + "%*j\t%" #OPNO "%/\n\t" \
> + ".option\tpic2") \
> + : "%*j\t%" #OPNO "%/")
Looks like the last bit could/should use MIPS_ABSOLUTE_JUMP.
> @@ -2923,3 +2960,5 @@ extern GTY(()) struct target_globals *mips16_globa
> with arguments ARGS. */
> #define PMODE_INSN(NAME, ARGS) \
> (Pmode == SImode ? NAME ## _si ARGS : NAME ## _di ARGS)
> +
> +#define UMIPS_12BIT_OFFSET_P(OFFSET) (IN_RANGE (OFFSET, -2048, 2047))
Please put this after #define LUI_INT, to keep the range checks together.
Thanks,
Richard
More information about the Gcc-patches
mailing list