| Message ID | 87k1zmxgbm.fsf@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [03/nn] Allow vector CONSTs | expand |
On 10/23/2017 05:18 AM, Richard Sandiford wrote: > This patch allows (const ...) wrappers to be used for rtx vector > constants, as an alternative to const_vector. This is useful > for SVE, where the number of elements isn't known until runtime. Right. It's constant, but not knowable at compile time. That seems an exact match for how we've used CONST. > > It could also be useful in future for fixed-length vectors, to > reduce the amount of memory needed to represent simple constants > with high element counts. However, one nice thing about keeping > it restricted to variable-length vectors is that there is never > any need to handle combinations of (const ...) and CONST_VECTOR. Yea, but is the memory consumption of these large vectors a real problem? I suspect, relative to other memory issues they're in the noise. > > > 2017-10-23 Richard Sandiford <richard.sandiford@linaro.org> > Alan Hayward <alan.hayward@arm.com> > David Sherwood <david.sherwood@arm.com> > > gcc/ > * doc/rtl.texi (const): Update description of address constants. > Say that vector constants are allowed too. > * common.md (E, F): Use CONSTANT_P instead of checking for > CONST_VECTOR. > * emit-rtl.c (gen_lowpart_common): Use const_vec_p instead of > checking for CONST_VECTOR. > * expmed.c (make_tree): Use build_vector_from_val for a CONST > VEC_DUPLICATE. > * expr.c (expand_expr_real_2): Check for vector modes instead > of checking for CONST_VECTOR. > * rtl.h (const_vec_p): New function. > (const_vec_duplicate_p): Check for a CONST VEC_DUPLICATE. > (unwrap_const_vec_duplicate): Handle them here too. My only worry here is code that is a bit loose in checking for a CONST, but not the innards and perhaps isn't prepared for for the new forms that appear inside the CONST. If we have such problems I'd expect it's in the targets as the targets have traditionally have had to validate the innards of a CONST to ensure it could be handled by the assembler/linker. Hmm, that may save the targets since they'd likely need an update to LEGITIMATE_CONSTANT_P to ever see these new forms. Presumably an aarch64 specific patch to recognize these as valid constants in LEGITIMATE_CONSTANT_P is in the works? OK for the trunk. jeff
Jeff Law <law@redhat.com> writes: > On 10/23/2017 05:18 AM, Richard Sandiford wrote: >> This patch allows (const ...) wrappers to be used for rtx vector >> constants, as an alternative to const_vector. This is useful >> for SVE, where the number of elements isn't known until runtime. > Right. It's constant, but not knowable at compile time. That seems an > exact match for how we've used CONST. > >> >> It could also be useful in future for fixed-length vectors, to >> reduce the amount of memory needed to represent simple constants >> with high element counts. However, one nice thing about keeping >> it restricted to variable-length vectors is that there is never >> any need to handle combinations of (const ...) and CONST_VECTOR. > Yea, but is the memory consumption of these large vectors a real > problem? I suspect, relative to other memory issues they're in the noise. Yeah, maybe not, especially since the elements themselves are shared. >> 2017-10-23 Richard Sandiford <richard.sandiford@linaro.org> >> Alan Hayward <alan.hayward@arm.com> >> David Sherwood <david.sherwood@arm.com> >> >> gcc/ >> * doc/rtl.texi (const): Update description of address constants. >> Say that vector constants are allowed too. >> * common.md (E, F): Use CONSTANT_P instead of checking for >> CONST_VECTOR. >> * emit-rtl.c (gen_lowpart_common): Use const_vec_p instead of >> checking for CONST_VECTOR. >> * expmed.c (make_tree): Use build_vector_from_val for a CONST >> VEC_DUPLICATE. >> * expr.c (expand_expr_real_2): Check for vector modes instead >> of checking for CONST_VECTOR. >> * rtl.h (const_vec_p): New function. >> (const_vec_duplicate_p): Check for a CONST VEC_DUPLICATE. >> (unwrap_const_vec_duplicate): Handle them here too. > My only worry here is code that is a bit loose in checking for a CONST, > but not the innards and perhaps isn't prepared for for the new forms > that appear inside the CONST. > > If we have such problems I'd expect it's in the targets as the targets > have traditionally have had to validate the innards of a CONST to ensure > it could be handled by the assembler/linker. Hmm, that may save the > targets since they'd likely need an update to LEGITIMATE_CONSTANT_P to > ever see these new forms. > > Presumably an aarch64 specific patch to recognize these as valid > constants in LEGITIMATE_CONSTANT_P is in the works? Yeah, via the const_vec_duplicate_p helper. For the default variable-length mode of SVE we use the (const ...) while for the fixed-length mode we use (const_vector ...) as normal. Advanced SIMD always uses (const_vector ...). > OK for the trunk. > > jeff Thanks, Richard
Index: gcc/doc/rtl.texi =================================================================== --- gcc/doc/rtl.texi 2017-10-23 11:41:22.176892260 +0100 +++ gcc/doc/rtl.texi 2017-10-23 11:41:39.185050437 +0100 @@ -1667,14 +1667,17 @@ Usually that is the only mode for which @findex const @item (const:@var{m} @var{exp}) -Represents a constant that is the result of an assembly-time -arithmetic computation. The operand, @var{exp}, is an expression that -contains only constants (@code{const_int}, @code{symbol_ref} and -@code{label_ref} expressions) combined with @code{plus} and -@code{minus}. However, not all combinations are valid, since the -assembler cannot do arbitrary arithmetic on relocatable symbols. +Wraps an rtx computation @var{exp} whose inputs and result do not +change during the execution of a thread. There are two valid uses. +The first is to represent a global or thread-local address calculation. +In this case @var{exp} should contain @code{const_int}, +@code{symbol_ref}, @code{label_ref} or @code{unspec} expressions, +combined with @code{plus} and @code{minus}. Any such @code{unspec}s +are target-specific and typically represent some form of relocation +operator. @var{m} should be a valid address mode. -@var{m} should be @code{Pmode}. +The second use of @code{const} is to wrap a vector operation. +In this case @var{exp} must be a @code{vec_duplicate} expression. @findex high @item (high:@var{m} @var{exp}) Index: gcc/common.md =================================================================== --- gcc/common.md 2017-10-23 11:40:11.431285821 +0100 +++ gcc/common.md 2017-10-23 11:41:39.184050436 +0100 @@ -80,14 +80,14 @@ (define_constraint "n" (define_constraint "E" "Matches a floating-point constant." (ior (match_test "CONST_DOUBLE_AS_FLOAT_P (op)") - (match_test "GET_CODE (op) == CONST_VECTOR + (match_test "CONSTANT_P (op) && GET_MODE_CLASS (GET_MODE (op)) == MODE_VECTOR_FLOAT"))) ;; There is no longer a distinction between "E" and "F". (define_constraint "F" "Matches a floating-point constant." (ior (match_test "CONST_DOUBLE_AS_FLOAT_P (op)") - (match_test "GET_CODE (op) == CONST_VECTOR + (match_test "CONSTANT_P (op) && GET_MODE_CLASS (GET_MODE (op)) == MODE_VECTOR_FLOAT"))) (define_constraint "X" Index: gcc/emit-rtl.c =================================================================== --- gcc/emit-rtl.c 2017-10-23 11:41:32.369050264 +0100 +++ gcc/emit-rtl.c 2017-10-23 11:41:39.186050437 +0100 @@ -1470,7 +1470,7 @@ gen_lowpart_common (machine_mode mode, r return gen_rtx_fmt_e (GET_CODE (x), int_mode, XEXP (x, 0)); } else if (GET_CODE (x) == SUBREG || REG_P (x) - || GET_CODE (x) == CONCAT || GET_CODE (x) == CONST_VECTOR + || GET_CODE (x) == CONCAT || const_vec_p (x) || CONST_DOUBLE_AS_FLOAT_P (x) || CONST_SCALAR_INT_P (x)) return lowpart_subreg (mode, x, innermode); Index: gcc/expmed.c =================================================================== --- gcc/expmed.c 2017-10-23 11:41:25.541909864 +0100 +++ gcc/expmed.c 2017-10-23 11:41:39.186050437 +0100 @@ -5246,7 +5246,15 @@ make_tree (tree type, rtx x) return fold_convert (type, make_tree (t, XEXP (x, 0))); case CONST: - return make_tree (type, XEXP (x, 0)); + { + rtx op = XEXP (x, 0); + if (GET_CODE (op) == VEC_DUPLICATE) + { + tree elt_tree = make_tree (TREE_TYPE (type), XEXP (op, 0)); + return build_vector_from_val (type, elt_tree); + } + return make_tree (type, op); + } case SYMBOL_REF: t = SYMBOL_REF_DECL (x); Index: gcc/expr.c =================================================================== --- gcc/expr.c 2017-10-23 11:41:24.408308073 +0100 +++ gcc/expr.c 2017-10-23 11:41:39.187050437 +0100 @@ -9429,7 +9429,7 @@ #define REDUCE_BIT_FIELD(expr) (reduce_b /* Careful here: if the target doesn't support integral vector modes, a constant selection vector could wind up smooshed into a normal integral constant. */ - if (CONSTANT_P (op2) && GET_CODE (op2) != CONST_VECTOR) + if (CONSTANT_P (op2) && !VECTOR_MODE_P (GET_MODE (op2))) { tree sel_type = TREE_TYPE (treeop2); machine_mode vmode Index: gcc/rtl.h =================================================================== --- gcc/rtl.h 2017-10-23 11:41:36.307050364 +0100 +++ gcc/rtl.h 2017-10-23 11:41:39.188050437 +0100 @@ -2749,12 +2749,22 @@ extern rtx shallow_copy_rtx (const_rtx C extern int rtx_equal_p (const_rtx, const_rtx); extern bool rtvec_all_equal_p (const_rtvec); +/* Return true if X is some form of vector constant. */ + +inline bool +const_vec_p (const_rtx x) +{ + return VECTOR_MODE_P (GET_MODE (x)) && CONSTANT_P (x); +} + /* Return true if X is a vector constant with a duplicated element value. */ inline bool const_vec_duplicate_p (const_rtx x) { - return GET_CODE (x) == CONST_VECTOR && rtvec_all_equal_p (XVEC (x, 0)); + return ((GET_CODE (x) == CONST_VECTOR && rtvec_all_equal_p (XVEC (x, 0))) + || (GET_CODE (x) == CONST + && GET_CODE (XEXP (x, 0)) == VEC_DUPLICATE)); } /* Return true if X is a vector constant with a duplicated element value. @@ -2764,11 +2774,16 @@ const_vec_duplicate_p (const_rtx x) inline bool const_vec_duplicate_p (T x, T *elt) { - if (const_vec_duplicate_p (x)) + if (GET_CODE (x) == CONST_VECTOR && rtvec_all_equal_p (XVEC (x, 0))) { *elt = CONST_VECTOR_ELT (x, 0); return true; } + if (GET_CODE (x) == CONST && GET_CODE (XEXP (x, 0)) == VEC_DUPLICATE) + { + *elt = XEXP (XEXP (x, 0), 0); + return true; + } return false; } @@ -2794,8 +2809,10 @@ vec_duplicate_p (T x, T *elt) inline T unwrap_const_vec_duplicate (T x) { - if (const_vec_duplicate_p (x)) - x = CONST_VECTOR_ELT (x, 0); + if (GET_CODE (x) == CONST_VECTOR && rtvec_all_equal_p (XVEC (x, 0))) + return CONST_VECTOR_ELT (x, 0); + if (GET_CODE (x) == CONST && GET_CODE (XEXP (x, 0)) == VEC_DUPLICATE) + return XEXP (XEXP (x, 0), 0); return x; }