[PATCH] rs6000: Add new __builtin_vsx_build_pair and __builtin_mma_build_acc built-ins

Peter Bergner bergner@linux.ibm.com
Thu Jun 10 15:43:05 GMT 2021


On 6/9/21 4:38 PM, Segher Boessenkool wrote:
> It is better if you *do* document the old names, but say "use the new
> stuff", I think?  Or is there so little material with the old names
> out there that no one will notice?

The latter.  There is only one user, but we want all new uses to use the
new built-in.  Plus, LLVM will be removing their documentation for that
built-in too, so we want to be consistent.



>> +      /* The ASSEMBLE builtin source operands are reversed in little-endian
>> +	 mode, so reorder them.  */
>> +      if (fcode == VSX_BUILTIN_ASSEMBLE_PAIR_INTERNAL && !WORDS_BIG_ENDIAN)
>> +	pat = GEN_FCN (icode) (op[0], op[2], op[1]);
>> +      else
>> +	pat = GEN_FCN (icode) (op[0], op[1], op[2]);
> 
> I think this reads simpler as
>       /* The ASSEMBLE builtin source operands are reversed in little-endian
> 	 mode, so reorder them.  */
>       if (fcode == VSX_BUILTIN_ASSEMBLE_PAIR_INTERNAL && !WORDS_BIG_ENDIAN)
> 	std::swap (op[1], op[2]);
>       pat = GEN_FCN (icode) (op[0], op[1], op[2]);
> do you agree?

Do I think C++ is simpler than plain C?  Is this a trick question? ;-)
If this is what you want, I'm fine with it.



> And
>       /* The ASSEMBLE builtin source operands are reversed in little-endian
> 	 mode, so reorder them.  */
>       if (fcode == MMA_BUILTIN_AS> +SEMBLE_ACC_INTERNAL && !WORDS_BIG_ENDIAN)
> 	{
> 	  std::swap (op[1], [op[4]);
> 	  std::swap (op[2], [op[3]);
> 	}
>       pat = GEN_FCN (icode) (op> +[0], op[1], op[2], op[3], op[4]);
> for that then of course.

Ok.


>> @@ -14151,7 +14161,8 @@ mma_init_builtins (void)
>>  	      if (gimple_func && mode == XOmode)
>>  		op[nopnds++] = build_pointer_type (vector_quad_type_node);
>>  	      else if (gimple_func && mode == OOmode
> 
> Pleae write the
>    && mode == OOmode
> on a new line as well then?

Will do.


>> +	      int index = WORDS_BIG_ENDIAN ? i: nvecs - 1 - i;
> 
> Space before colon.

Ah, thanks for catching that!



> Okay for trunk and 11 with at least that space fixed.  Thanks!

Ok, I'll retest with the above (but still removing the assemble
built-in documentation) and push it if it's clean.  Thanks!

Peter




More information about the Gcc-patches mailing list