"Moore, Catherine" <catherine_mo...@mentor.com> writes:
>> -----Original Message-----
>> From: Richard Sandiford [mailto:rdsandif...@googlemail.com]
>> Sent: Monday, March 04, 2013 3:54 PM
>> To: Moore, Catherine
>> Cc: gcc-patches@gcc.gnu.org; Rozycki, Maciej
>> Subject: Re: FW: [PATCH] [MIPS] microMIPS gcc support
>>
>> "Moore, Catherine" <catherine_mo...@mentor.com> writes:
>> > Hi Richard,
>> - Predicates should always check the code though. E.g.:
>>
>> (define_predicate "umips_addius5_imm"
>> (and (match_code "const_int")
>> (match_test "IN_RANGE (INTVAL (op), -8, 7)")))
>>
>> - In general, please try to make the names of the predicates as generic
>> as possible. There's nothing really add-specific about the predicate
>> above. Or microMIPS-specific either really: some of these predicates
>> are probably going to be useful for MIPS16 too.
>>
>> The existing MIPS16 functions follow the convention:
>>
>> "n" if negated (optional)
>> + "s" or "u" for signed vs. unsigned
>> + "imm"
>> + number of significant bits
>> + "_"
>> + multiplication factor or, er, "b" for "+1"...
>>
>> It might be nice to have a similar convention for microMIPS.
>> The choices there are a bit more exotic, so please feel free to
>> diverge from the MIPS16 one above; we can switch MIPS16 over once
>> the microMIPS one is settled. In fact, a new convention that's
>> compact enough to be used in both predicate and constraint names
>> would be great. E.g. for the umips_addius5_imm predicate above,
>> a name like "Ys5" would be easier to remember than "Zo"/"Yo".
>
> How compact would you consider "compact enough"? I would need to change
> the existing "Y" constraints as well.
Argh, sorry, I'd forgotten about that restriction.
We have a few internal-only undocumented constraints that aren't used much,
so we should be able to move them to the "Y" space instead. The patch
below does this for "T" and "U". Then we could use "U" for new, longer
constraints.
> I think trying to invent some convention with less than four letter will
> be difficult and even with four, I doubt it could be uniformly followed.
> I think we could get descriptive with four, however.
> Let me know what you think.
Four sounds good. Here's one idea:
U<type><factor><bits>
where <type> is:
s for signed
u for unsigned
d for decremented unsigned (-1 ... N)
i for incremented unsigned (1 ... N)
where <factor> is:
b for "byte" (*1)
h for "halfwords" (*2)
w for "words" (*4)
d for "doublewords" (*8) -- useful for 64-bit MIPS16 but probably not
needed for 32-bit microMIPS
and where <bits> is the number of bits. <type> and <factor> could be
replaced with an ad-hoc two-letter combination for special cases.
E.g. "Uas9" ("add stack") for ADDISUP.
Just a suggestion though. I'm not saying these names are totally intuitive
or anything, but they should at least be better than arbitrary letters.
Also, <bits> could be two digits if necessary, or we could just use
hex digits.
We could have:
/* Return true if X fits within an unsigned field of BITS bits that is
shifted left SHIFT bits before being used. */
static inline bool
mips_unsigned_immediate_p (unsigned HOST_WIDE_INT x, int bits, int shift = 0)
{
return (x & ((1 << shift) - 1)) == 0 && x < (1 << (shift + bits));
}
/* Return true if X fits within a signed field of BITS bits that is
shifted left SHIFT bits before being used. */
static inline bool
mips_signed_immediate_p (unsigned HOST_WIDE_INT x, int bits, int shift = 0)
{
x += 1 << (bits + shift - 1);
return mips_unsigned_immediate_p (x, bits, shift);
}
The 'd' and 'i' cases would pass a biased X to mips_unsigned_immediate_p.
I'll apply the patch below once 4.9 starts.
Thanks,
Richard
gcc/
* config/mips/constraints.md (T): Rename to...
(Yf): ...this.
(U): Rename to...
(Yd): ...this.
* config/mips/mips.md (*movdi_64bit, *movdi_64bit_mips16)
(*mov<mode>_internal, *mov<mode>_mips16): Update accordingly.
Index: gcc/config/mips/constraints.md
===================================================================
--- gcc/config/mips/constraints.md 2013-02-25 21:45:10.000000000 +0000
+++ gcc/config/mips/constraints.md 2013-03-05 08:22:36.687354771 +0000
@@ -170,22 +170,6 @@ (define_constraint "S"
(and (match_operand 0 "call_insn_operand")
(match_test "CONSTANT_P (op)")))
-(define_constraint "T"
- "@internal
- A constant @code{move_operand} that cannot be safely loaded into @code{$25}
- using @code{la}."
- (and (match_operand 0 "move_operand")
- (match_test "CONSTANT_P (op)")
- (match_test "mips_dangerous_for_la25_p (op)")))
-
-(define_constraint "U"
- "@internal
- A constant @code{move_operand} that can be safely loaded into @code{$25}
- using @code{la}."
- (and (match_operand 0 "move_operand")
- (match_test "CONSTANT_P (op)")
- (not (match_test "mips_dangerous_for_la25_p (op)"))))
-
(define_memory_constraint "W"
"@internal
A memory address based on a member of @code{BASE_REG_CLASS}. This is
@@ -220,6 +204,22 @@ (define_constraint "Yb"
"@internal"
(match_operand 0 "qi_mask_operand"))
+(define_constraint "Yd"
+ "@internal
+ A constant @code{move_operand} that can be safely loaded into @code{$25}
+ using @code{la}."
+ (and (match_operand 0 "move_operand")
+ (match_test "CONSTANT_P (op)")
+ (not (match_test "mips_dangerous_for_la25_p (op)"))))
+
+(define_constraint "Yf"
+ "@internal
+ A constant @code{move_operand} that cannot be safely loaded into @code{$25}
+ using @code{la}."
+ (and (match_operand 0 "move_operand")
+ (match_test "CONSTANT_P (op)")
+ (match_test "mips_dangerous_for_la25_p (op)")))
+
(define_constraint "Yh"
"@internal"
(match_operand 0 "hi_mask_operand"))
Index: gcc/config/mips/mips.md
===================================================================
--- gcc/config/mips/mips.md 2013-02-25 21:45:10.000000000 +0000
+++ gcc/config/mips/mips.md 2013-03-05 08:25:31.762333026 +0000
@@ -4268,7 +4268,7 @@ (define_insn "*movdi_32bit_mips16"
(define_insn "*movdi_64bit"
[(set (match_operand:DI 0 "nonimmediate_operand" "=d,d,e,d,m,*f,*f,*d,*m,*a,*d,*B*C*D,*B*C*D,*d,*m")
- (match_operand:DI 1 "move_operand" "d,U,T,m,dJ,*d*J,*m,*f,*f,*J*d,*a,*d,*m,*B*C*D,*B*C*D"))]
+ (match_operand:DI 1 "move_operand" "d,Yd,Yf,m,dJ,*d*J,*m,*f,*f,*J*d,*a,*d,*m,*B*C*D,*B*C*D"))]
"TARGET_64BIT && !TARGET_MIPS16
&& (register_operand (operands[0], DImode)
|| reg_or_0_operand (operands[1], DImode))"
@@ -4278,7 +4278,7 @@ (define_insn "*movdi_64bit"
(define_insn "*movdi_64bit_mips16"
[(set (match_operand:DI 0 "nonimmediate_operand" "=d,y,d,d,d,d,d,d,m,*d")
- (match_operand:DI 1 "move_operand" "d,d,y,K,N,U,kf,m,d,*a"))]
+ (match_operand:DI 1 "move_operand" "d,d,y,K,N,Yd,kf,m,d,*a"))]
"TARGET_64BIT && TARGET_MIPS16
&& (register_operand (operands[0], DImode)
|| register_operand (operands[1], DImode))"
@@ -4346,7 +4346,7 @@ (define_expand "mov<mode>"
(define_insn "*mov<mode>_internal"
[(set (match_operand:IMOVE32 0 "nonimmediate_operand" "=d,d,e,d,m,*f,*f,*d,*m,*d,*z,*a,*d,*B*C*D,*B*C*D,*d,*m")
- (match_operand:IMOVE32 1 "move_operand" "d,U,T,m,dJ,*d*J,*m,*f,*f,*z,*d,*J*d,*a,*d,*m,*B*C*D,*B*C*D"))]
+ (match_operand:IMOVE32 1 "move_operand" "d,Yd,Yf,m,dJ,*d*J,*m,*f,*f,*z,*d,*J*d,*a,*d,*m,*B*C*D,*B*C*D"))]
"!TARGET_MIPS16
&& (register_operand (operands[0], <MODE>mode)
|| reg_or_0_operand (operands[1], <MODE>mode))"
@@ -4356,7 +4356,7 @@ (define_insn "*mov<mode>_internal"
(define_insn "*mov<mode>_mips16"
[(set (match_operand:IMOVE32 0 "nonimmediate_operand" "=d,y,d,d,d,d,d,d,m,*d")
- (match_operand:IMOVE32 1 "move_operand" "d,d,y,K,N,U,kf,m,d,*a"))]
+ (match_operand:IMOVE32 1 "move_operand" "d,d,y,K,N,Yd,kf,m,d,*a"))]
"TARGET_MIPS16
&& (register_operand (operands[0], <MODE>mode)
|| register_operand (operands[1], <MODE>mode))"
