Message ID | 56A4AD68.5090703@redhat.com |
---|---|
State | New |
Headers | show |
Hi Richard, On 24/01/16 10:54, Richard Henderson wrote: > This looks to be an incomplete transition of the aarch64 backend to CONST_WIDE_INT. I haven't checked to see if it's a regression from gcc5, but I suspect not, since there should have been similar checks for CONST_DOUBLE. > FWIW, I defined TARGET_SUPPORTS_WIDE_INT for aarch64 on trunk and the GCC 5 branch in order to fix PR 68129. > This is probably gcc7 fodder, but it helped me debug another TImode PR. > > > r~ + case CONST_WIDE_INT: + *cost = 0; + for (unsigned int n = CONST_WIDE_INT_NUNITS(x), i = 0; i < n; ++i) + { + unsigned HOST_WIDE_INT e = CONST_WIDE_INT_ELT(x, i); + if (e != 0) + *cost += COSTS_N_INSNS (aarch64_internal_mov_immediate + (NULL_RTX, GEN_INT (e), false, DImode)); + } + return true; + We usually avoid creating intermediate rtxes in the cost function because it can potentially be called many times during compilation and we want to avoid creating too many short-lived objects, though I suppose there's no way getting around this one (the GEN_INT call). Thanks, Kyrill
On 01/25/2016 01:32 AM, Kyrill Tkachov wrote: > + case CONST_WIDE_INT: > + *cost = 0; > + for (unsigned int n = CONST_WIDE_INT_NUNITS(x), i = 0; i < n; ++i) > + { > + unsigned HOST_WIDE_INT e = CONST_WIDE_INT_ELT(x, i); > + if (e != 0) > + *cost += COSTS_N_INSNS (aarch64_internal_mov_immediate > + (NULL_RTX, GEN_INT (e), false, DImode)); > + } > + return true; > + > > We usually avoid creating intermediate rtxes in the cost function because > it can potentially be called many times during compilation and we want to avoid > creating too many short-lived objects, though I suppose there's no way getting > around this one (the GEN_INT call). Well, it's only aarch64_internal_mov_immediate -- we could change the interface to provide the HOST_WIDE_INT value directly. But that was more than I wanted to do for enabling splittable TImode constants. r~
On Sun, Jan 24, 2016 at 02:54:32AM -0800, Richard Henderson wrote: > This looks to be an incomplete transition of the aarch64 backend to > CONST_WIDE_INT. I haven't checked to see if it's a regression from > gcc5, but I suspect not, since there should have been similar checks > for CONST_DOUBLE. > > This is probably gcc7 fodder, but it helped me debug another TImode PR. When the time comes, this is OK. Thanks, James > * config/aarch64/aarch64.c (aarch64_rtx_costs): Handle CONST_WIDE_INT. > (aarch64_legitimate_constant_p): Accept CONST_SCALAR_INT_P. > * config/aarch64/predicates.md (aarch64_movti_operand): Accept > const_wide_int and const_scalar_int_operand. > (aarch64_reg_or_imm): Likewise. > > > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > index df3dec0..38c7443 100644 > --- a/gcc/config/aarch64/aarch64.c > +++ b/gcc/config/aarch64/aarch64.c > @@ -6227,6 +6227,17 @@ aarch64_rtx_costs (rtx x, machine_mode mode, int outer ATTRIBUTE_UNUSED, > } > return true; > > + case CONST_WIDE_INT: > + *cost = 0; > + for (unsigned int n = CONST_WIDE_INT_NUNITS(x), i = 0; i < n; ++i) > + { > + unsigned HOST_WIDE_INT e = CONST_WIDE_INT_ELT(x, i); > + if (e != 0) > + *cost += COSTS_N_INSNS (aarch64_internal_mov_immediate > + (NULL_RTX, GEN_INT (e), false, DImode)); > + } > + return true; > + > case CONST_DOUBLE: > if (speed) > { > @@ -9400,6 +9411,9 @@ aarch64_legitimate_constant_p (machine_mode mode, rtx x) > && aarch64_valid_symref (XEXP (x, 0), GET_MODE (XEXP (x, 0)))) > return true; > > + if (CONST_SCALAR_INT_P (x)) > + return true; > + > return aarch64_constant_address_p (x); > } > > diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md > index e96dc00..3eb33fa 100644 > --- a/gcc/config/aarch64/predicates.md > +++ b/gcc/config/aarch64/predicates.md > @@ -217,15 +217,15 @@ > (match_test "aarch64_mov_operand_p (op, mode)"))))) > > (define_predicate "aarch64_movti_operand" > - (and (match_code "reg,subreg,mem,const_int") > + (and (match_code "reg,subreg,mem,const_int,const_wide_int") > (ior (match_operand 0 "register_operand") > (ior (match_operand 0 "memory_operand") > - (match_operand 0 "const_int_operand"))))) > + (match_operand 0 "const_scalar_int_operand"))))) > > (define_predicate "aarch64_reg_or_imm" > - (and (match_code "reg,subreg,const_int") > + (and (match_code "reg,subreg,const_int,const_wide_int") > (ior (match_operand 0 "register_operand") > - (match_operand 0 "const_int_operand")))) > + (match_operand 0 "const_scalar_int_operand")))) > > ;; True for integer comparisons and for FP comparisons other than LTGT or UNEQ. > (define_special_predicate "aarch64_comparison_operator"
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index df3dec0..38c7443 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -6227,6 +6227,17 @@ aarch64_rtx_costs (rtx x, machine_mode mode, int outer ATTRIBUTE_UNUSED, } return true; + case CONST_WIDE_INT: + *cost = 0; + for (unsigned int n = CONST_WIDE_INT_NUNITS(x), i = 0; i < n; ++i) + { + unsigned HOST_WIDE_INT e = CONST_WIDE_INT_ELT(x, i); + if (e != 0) + *cost += COSTS_N_INSNS (aarch64_internal_mov_immediate + (NULL_RTX, GEN_INT (e), false, DImode)); + } + return true; + case CONST_DOUBLE: if (speed) { @@ -9400,6 +9411,9 @@ aarch64_legitimate_constant_p (machine_mode mode, rtx x) && aarch64_valid_symref (XEXP (x, 0), GET_MODE (XEXP (x, 0)))) return true; + if (CONST_SCALAR_INT_P (x)) + return true; + return aarch64_constant_address_p (x); } diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md index e96dc00..3eb33fa 100644 --- a/gcc/config/aarch64/predicates.md +++ b/gcc/config/aarch64/predicates.md @@ -217,15 +217,15 @@ (match_test "aarch64_mov_operand_p (op, mode)"))))) (define_predicate "aarch64_movti_operand" - (and (match_code "reg,subreg,mem,const_int") + (and (match_code "reg,subreg,mem,const_int,const_wide_int") (ior (match_operand 0 "register_operand") (ior (match_operand 0 "memory_operand") - (match_operand 0 "const_int_operand"))))) + (match_operand 0 "const_scalar_int_operand"))))) (define_predicate "aarch64_reg_or_imm" - (and (match_code "reg,subreg,const_int") + (and (match_code "reg,subreg,const_int,const_wide_int") (ior (match_operand 0 "register_operand") - (match_operand 0 "const_int_operand")))) + (match_operand 0 "const_scalar_int_operand")))) ;; True for integer comparisons and for FP comparisons other than LTGT or UNEQ. (define_special_predicate "aarch64_comparison_operator"