Re: [aarch64][PATCH v2] Disable reg offset in quad-word store for Falkor

[Siddhesh Poyarekar <siddhesh at sourceware dot org>]

On Wednesday 24 January 2018 05:50 PM, Kyrill Tkachov wrote:
> I would tend towards making the costs usage more intelligent and
> differentiating
> between loads and stores but I agree that is definitely GCC 9 material.
> Whether this approach is an acceptable stopgap for GCC 8 is up to the
> aarch64 maintainers
> and will depend, among other things, on the impact it has on generic
> (non-Falkor) codegen.
> A good experiment to help this approach would be to compile a large
> codebase (for example CPU2017)
> with a non-Falkor -mcpu setting and make sure that there are no assembly
> changes (or minimal).
> This would help justify the aarch64.md constraint changes.

Thanks, I'll verify with CPU2017.

> file paths don't need the gcc/ because the ChangeLog file is already in
> the gcc/ directory
> 
>>         gcc/testsuite/
>>         * gcc/testsuite/gcc.target/aarch64/pr82533.c: New test case.
> 
> Similarly, you don't need the gcc/testsuite/ prefix.
> Also, since you have a bugzilla PR entry please reference it in the
> ChangeLog
> right above the file list:
> <tab>PR target/82533
> 
> That way when the patch is committed the SVN hooks will pick it up
> automagically and update bugzilla.

Ugh, sorry, I was tardy about this - I'll fix it up.

>> @@ -5530,6 +5530,16 @@ aarch64_classify_address (struct
>> aarch64_address_info *info,
>>                                  || vec_flags == VEC_ADVSIMD
>>                                  || vec_flags == VEC_SVE_DATA));
>>
>> +  /* Avoid register indexing for 128-bit stores when the
>> +     AARCH64_EXTRA_TUNE_SLOW_REGOFFSET_QUADWORD_STORE option is set.  */
>> +  if (!optimize_size
>> +      && type == ADDR_QUERY_STR
>> +      && (aarch64_tune_params.extra_tuning_flags
>> +         & AARCH64_EXTRA_TUNE_SLOW_REGOFFSET_QUADWORD_STORE)
>> +      && (mode == TImode || mode == TFmode
>> +         || aarch64_vector_data_mode_p (mode)))
>> +    allow_reg_index_p = false;
> 
> The aarch64_classify_vector_mode code has been reworked recently for SVE
> so I'm not entirely
> up to date with its logic, but I believe that
> "aarch64_classify_vector_mode (mode)" will
> allow 64-bit vector modes, which would not be using the 128-bit Q
> register, so you may be disabling
> register indexing for D-register memory stores.

I check this and fix the condition if necessary.

Thanks,
Siddhesh