[AArch64,2/3] Correct type attribute for mul and mneg instructions

Hi all,

In the AAarch64 ISA the MUL and MNEG instructions are actually aliases of MADD and MSUB.
Therefore they should have the type attribute mla, rather than mul, which should only be used
for AArch32 32-bit multiplication instructions.

This will ensure more consistent scheduling decisions.

Bootstrapped and tested on aarch64-none-linux-gnu.

Ok for trunk?

Thanks,
Kyrill

2018-11-26  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * config/aarch64/aarch64.md (mul<mode>3): Change type to mla.
     (*mulsi3_uxtw): Likewise.
     (*mul<mode>_neg): Likewise.
     (*mulsi_neg_uxtw): Likewise.

On Mon, Nov 26, 2018 at 11:36:43AM -0600, Kyrill Tkachov wrote:
> Hi all,
> 
> In the AAarch64 ISA the MUL and MNEG instructions are actually aliases of MADD and MSUB.
> Therefore they should have the type attribute mla, rather than mul, which should only be used
> for AArch32 32-bit multiplication instructions.
> 
> This will ensure more consistent scheduling decisions.
> 
> Bootstrapped and tested on aarch64-none-linux-gnu.
> 
> Ok for trunk?

OK in principle. Did you audit the pipeline models to check this doesn't
change scheduling class in an undesirable way for any of our supported
targets? OK if so, if not can you run that audit and figure out the right
thing to do to resolve it.

Thanks,
James

On 28/11/18 16:37, James Greenhalgh wrote:
> On Mon, Nov 26, 2018 at 11:36:43AM -0600, Kyrill Tkachov wrote:
>> Hi all,
>>
>> In the AAarch64 ISA the MUL and MNEG instructions are actually aliases of MADD and MSUB.
>> Therefore they should have the type attribute mla, rather than mul, which should only be used
>> for AArch32 32-bit multiplication instructions.
>>
>> This will ensure more consistent scheduling decisions.
>>
>> Bootstrapped and tested on aarch64-none-linux-gnu.
>>
>> Ok for trunk?
> OK in principle. Did you audit the pipeline models to check this doesn't
> change scheduling class in an undesirable way for any of our supported
> targets? OK if so, if not can you run that audit and figure out the right
> thing to do to resolve it.

This has a change in 3 models.

In saphira.md this changes the reservation of these instructions
from saphira_muldiv_4_x_mul to saphira_muldiv_4_x_mla.
Both have the same characteristics (latency etc) but they differ in
their bypass behaviour: saphira_muldiv_4_x_mla can receive a bypass from saphira_muldiv_4_x_mul,
but not the other way around.

So with this change the MUL and MNEG will now be able to receive an early forwarded result from other multiply/multiply-accumulate operations,
whereas before this was only done on MADD instructions.
There is a similar situation in falkor.md.
Sameera, can you share whether this new behaviour is appropriate for Falkor and Saphira?
I note that if the forwarding only occurs on the accumulator operand, the bypass should be guarded on aarch_accumulator_forwarding

In thunderx2t99.md the reservation changes from thunderx2t99_mul to thunderx2t99_madd.

Steve, can you share whether the AArch64 MUL and MNEG instructions really do have different latencies and reservations from MADD and MSUB
on Thunderx2? If so, then this change is wrong :( and we'll want to model these forms differently indeed.

Thanks,
Kyrill


> Thanks,
> James
>

On Fri, 2018-11-30 at 15:37 +0000, Kyrill Tkachov wrote:
> 
> In thunderx2t99.md the reservation changes from thunderx2t99_mul to
> thunderx2t99_madd.
> 
> Steve, can you share whether the AArch64 MUL and MNEG instructions
> really do have different latencies and reservations from MADD and
> MSUB
> on Thunderx2? If so, then this change is wrong :( and we'll want to
> model these forms differently indeed.
> 
> Thanks,
> Kyrill

According to the thunderx2 documents I looked at, the mul/mneg
instructions do have the same latencies as madd/msub.  So this
patch is OK from that standpoint and fixes an existing problem
on Thunderx2.

Steve Ellcey