RE: [PATCH] [Aarch64] Variable shift count truncation issues

[Michael Collison <Michael dot Collison at arm dot com>]

Updated the patch to more fully address the reload_completed issues raised by Richard, to ensure that the SIMD patterns are not used for shifts and fixed a code generation bug.

Okay for trunk?

Bootstrapped and tested on aarch64-linux-gnu. Okay for trunk?

2017-05-22  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
	    Michael Collison <michael.collison@arm.com>

	PR target/70119
	* config/aarch64/aarch64.md (*aarch64_<optab>_reg_<mode>3_mask1):
	New pattern.
	(*aarch64_reg_<mode>3_neg_mask2): New pattern.
	(*aarch64_reg_<mode>3_minus_mask): New pattern.
	(*aarch64_<optab>_reg_di3_mask2): New pattern.
	* config/aarch64/aarch64.c (aarch64_rtx_costs): Account for cost
	of shift when the shift amount is masked with constant equal to
	the size of the mode.
	* config/aarch64/predicates.md (subreg_lowpart_operator): New
	predicate.


2016-05-22  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
	    Michael Collison <michael.collison@arm.com>

	PR target/70119
	* gcc.target/aarch64/var_shift_mask_1.c: New test.

-----Original Message-----
From: Michael Collison 
Sent: Tuesday, May 23, 2017 4:01 PM
To: 'Richard Sandiford' <richard.sandiford@linaro.org>; Wilco Dijkstra <Wilco.Dijkstra@arm.com>; 'Christophe Lyon' <christophe.lyon@linaro.org>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>; nd <nd@arm.com>
Subject: RE: [PATCH] [Aarch64] Variable shift count truncation issues

I updated the patch to resolve the big endian issues as recommended by Richard. I also changed all uses of 'can_create_pseudo_p' to '!reload_completed' to make the intent clearer.

Bootstrapped and tested on aarch64-linux-gnu. Okay for trunk?

2017-05-22  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
	    Michael Collison <michael.collison@arm.com>

	PR target/70119
	* config/aarch64/aarch64.md (*aarch64_<optab>_reg_<mode>3_mask1):
	New pattern.
	(*aarch64_reg_<mode>3_neg_mask2): New pattern.
	(*aarch64_reg_<mode>3_minus_mask): New pattern.
	(*aarch64_<optab>_reg_di3_mask2): New pattern.
	* config/aarch64/aarch64.c (aarch64_rtx_costs): Account for cost
	of shift when the shift amount is masked with constant equal to
	the size of the mode.
	* config/aarch64/predicates.md (subreg_lowpart_operator): New
	predicate.


2016-05-22  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
	    Michael Collison <michael.collison@arm.com>

	PR target/70119
	* gcc.target/aarch64/var_shift_mask_1.c: New test.
-----Original Message-----
From: Richard Sandiford [mailto:richard.sandiford@linaro.org]
Sent: Tuesday, May 23, 2017 7:25 AM
To: Wilco Dijkstra <Wilco.Dijkstra@arm.com>
Cc: Michael Collison <Michael.Collison@arm.com>; GCC Patches <gcc-patches@gcc.gnu.org>; nd <nd@arm.com>
Subject: Re: [PATCH] [Aarch64] Variable shift count truncation issues

Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
> Richard Sandiford wrote:
>
>> Insn patterns shouldn't check can_create_pseudo_p, because there's no 
>> guarantee that the associated split happens before RA.  In this case 
>> it should be safe to reuse operand 0 after RA if you change it to:
>
> The goal is to only create and split this pattern before register allocation.
> It's a transient pattern, combine creates it, and it is split immediately after.
>
> Maybe !reload_completed is clearer as that is what several other 
> define_insn_and_split patterns do?

But the concept of a transient pattern doesn't really exist.  We shouldn't rely for correctness on a split being applied before RA.

If all we want to do is match and split something that combine generates, it would be better to do it as a pure define_split.

Thanks,
Richard

Attachment: pr5546v4.patch
Description: pr5546v4.patch