[MIPS][LS2][2/5] Vector intrinsics

[Maxim Kuvyrkov]

Richard,

Thanks for the review, I'll post an updated patch when it's finished. 
Below is a couple of notes.

> 
> Maxim Kuvyrkov <maxim@codesourcery.com> writes:
>> Index: gcc/testsuite/lib/target-supports.exp
>> ===================================================================
>> --- gcc/testsuite/lib/target-supports.exp	(revision 62)
>> +++ gcc/testsuite/lib/target-supports.exp	(working copy)
>> @@ -1252,6 +1252,17 @@ proc check_effective_target_arm_neon_hw 
>>      } "-mfpu=neon -mfloat-abi=softfp"]
>>  }
>>  
>> +# Return 1 if this a Loongson-2E or -2F target using an ABI that supports
>> +# the Loongson vector modes.
>> +
>> +proc check_effective_target_mips_loongson { } {
>> +    return [check_no_compiler_messages loongson assembly {
>> +	#if !defined(_MIPS_LOONGSON_VECTOR_MODES)
>> +	#error FOO
>> +	#endif
>> +    }]
>> +}
>> +
> 
> I think this is a poor choice of name for a user-visible macro.  "modes"
> are an internal gcc concept, and your .h-based API shields the user from
> the "__attribute__"s needed to construct the types.

Does "_MIPS_LOONGSON_VECTOR_INSNS" look good?

> 
>> +;; Move patterns.
>> +
>> +;; Expander to legitimize moves involving values of vector modes.
>> +(define_expand "mov<mode>"
>> +  [(set (match_operand:VWHB 0 "nonimmediate_operand")
>> +	(match_operand:VWHB 1 "move_operand"))]
>> +  ""
>> +{
>> +  if (mips_legitimize_move (<MODE>mode, operands[0], operands[1]))
>> +    DONE;
>> +})
> 
> Hmm.  This is probably going to cause problems if other ASEs use
> the same modes in future, but I guess this is OK until then.
> 
> Local style is not to have predicates for move expanders.
> The predicates aren't checked, and I think it's confusing
> to have an expander with "move_operand" as its predicate,
> and to then call a function (mips_legitimize_move) that
> deals with non-move_operands.  So:
> 
>   [(set (match_operand:VWHB 0)
> 	(match_operand:VWHB 1)]

I don't mean to challenge local style, but it is valid and useful to 
specify predicates for operands in define_expand as well as for the 
whole expand.  At least, this is what GCC Internals say:

<sic>
14.15 Defining RTL Sequences for Code Generation

...

<In the context of define_expand, not define_insn>:
The RTL template, in addition to controlling generation of RTL insns, 
also describes the operands that need to be specified when this pattern 
is used. In particular, it gives a predicate for each operand.

A true operand, which needs to be specified in order to generate RTL 
from the pattern, should be described with a match_operand in its first 
occurrence in the RTL template. This enters information on the operandÂ’s 
predicate into the tables that record such things. GCC uses the 
information to preload the operand into a register if that is required 
for valid RTL code. If the operand is referred to more than once, 
subsequent references should use match_dup.
</sic>

Am I missing something?


--
Maxim