| Message ID | AM3PR08MB0088B5BC9E5DDED17C607D1B836F0@AM3PR08MB0088.eurprd08.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
On 04/22/16 10:35, Wilco Dijkstra wrote:
> OK for trunk?
LGTM
On Fri, Apr 22, 2016 at 03:35:42PM +0000, Wilco Dijkstra wrote: > SIMD operations like combine prefer to have their operands in FP registers, > so increase the cost of integer registers slightly to avoid unnecessary > int<->FP moves. This improves register allocation of scalar SIMD operations. I really don't like [1][2][3] this technique of attempting to work around register allocator issues using the disparaging mechanisms. If we take this, our set of patterns using disparaging becomes: aarch64_combinez aarch64_combinez_be aarch64_simd_mov movhf_aarch64 movsf_aarch64 movdf_aarch64 movtf_aarch64 xor_one_cmpl So doing this would be in line with other move operations, but is still a workaround to deeper issues. The patch is OK, on that justification, but I'd like not to set a precedent for using "?" rather than looking to find the underlying issue. Thanks, James --- [1] Re: [AArch64] Implement ADD in vector registers for 32-bit scalar values. https://gcc.gnu.org/ml/gcc-patches/2014-03/msg01627.html [2] Re: [PATCH AArch64 1/3] Don't disparage add/sub in SIMD registers https://gcc.gnu.org/ml/gcc-patches/2014-08/msg01332.html [3] Re: [PATCH AArch64 3/3] Fix XOR_one_cmpl pattern; add SIMD-reg variants for BIC,ORN,EON https://gcc.gnu.org/ml/gcc-patches/2014-08/msg01278.html > > OK for trunk? > > ChangeLog: > 2016-04-22 Wilco Dijkstra <wdijkstr@arm.com> > > * gcc/config/aarch64/aarch64-simd.md (aarch64_combinez): > Add ? to integer variant. > (aarch64_combinez_be): Likewise. > > -- >
James Greenhalgh wrote: > I really don't like [1][2][3] this technique of attempting to work around > register allocator issues using the disparaging mechanisms. I don't see the issue as it is a standard mechanism to describe higher cost to the register allocator. On the other had the use of '*' is almost always incorrect, leading to bad allocations and inefficient code. > So doing this would be in line with other move operations, but is still > a workaround to deeper issues. > > The patch is OK, on that justification, but I'd like not to set a > precedent for using "?" rather than looking to find the underlying issue. The underlying issue is well known - the register allocator cost code assumes all alternatives in an instruction have equal cost. If we used a distinct scalar vector type for scalar vectors (rather than reusing SI/DI) then we could drop all of the '*' and likely most of the '?' from the md description. Wilco
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md index e1f5682165cd22ca7d31643b8f4e7f631d99c2d8..d3830838867eec2098b71eb46b7343d0155acf7e 100644 --- a/gcc/config/aarch64/aarch64-simd.md +++ b/gcc/config/aarch64/aarch64-simd.md @@ -2645,7 +2645,7 @@ (define_insn "*aarch64_combinez<mode>" [(set (match_operand:<VDBL> 0 "register_operand" "=w,w,w") (vec_concat:<VDBL> - (match_operand:VD_BHSI 1 "general_operand" "w,r,m") + (match_operand:VD_BHSI 1 "general_operand" "w,?r,m") (match_operand:VD_BHSI 2 "aarch64_simd_imm_zero" "Dz,Dz,Dz")))] "TARGET_SIMD && !BYTES_BIG_ENDIAN" "@ @@ -2661,7 +2661,7 @@ [(set (match_operand:<VDBL> 0 "register_operand" "=w,w,w") (vec_concat:<VDBL> (match_operand:VD_BHSI 2 "aarch64_simd_imm_zero" "Dz,Dz,Dz") - (match_operand:VD_BHSI 1 "general_operand" "w,r,m")))] + (match_operand:VD_BHSI 1 "general_operand" "w,?r,m")))] "TARGET_SIMD && BYTES_BIG_ENDIAN" "@ mov\\t%0.8b, %1.8b