On Fri, Jan 05, 2018 at 11:26:59AM +0000, Richard Sandiford wrote: > Ping. Here's the patch updated to apply on top of the v8.4 and > __builtin_load_no_speculate support. > > Richard Sandiford <richard.sandif...@linaro.org> writes: > > This patch switches the AArch64 port to use 2 poly_int coefficients > > and updates code as necessary to keep it compiling. > > > > One potentially-significant change is to > > aarch64_hard_regno_caller_save_mode. The old implementation > > was written in a pretty conservative way: it changed the default > > behaviour for single-register values, but used the default handling > > for multi-register values. > > > > I don't think that's necessary, since the interesting cases for this > > macro are usually the single-register ones. Multi-register modes take > > up the whole of the constituent registers and the move patterns for all > > multi-register modes should be equally good. > > > > Using the original mode for multi-register cases stops us from using > > SVE modes to spill multi-register NEON values. This was caught by > > gcc.c-torture/execute/pr47538.c. > > > > Also, aarch64_shift_truncation_mask used GET_MODE_BITSIZE - 1. > > GET_MODE_UNIT_BITSIZE - 1 is equivalent for the cases that it handles > > (which are all scalars), and I think it's more obvious, since if we ever > > do use this for elementwise shifts of vector modes, the mask will depend > > on the number of bits in each element rather than the number of bits in > > the whole vector.
This is OK for trunk, with whatever modifications are needed to make the rebase work. I do have one question and one minor comment; you can assume that if you do make modifications in response to these that the patch is still OK. > Index: gcc/config/aarch64/aarch64.c > =================================================================== > --- gcc/config/aarch64/aarch64.c 2018-01-05 11:24:44.647408566 +0000 > +++ gcc/config/aarch64/aarch64.c 2018-01-05 11:24:44.867399574 +0000 > @@ -2262,8 +2258,12 @@ aarch64_pass_by_reference (cumulative_ar > int nregs; > > /* GET_MODE_SIZE (BLKmode) is useless since it is 0. */ > - size = (mode == BLKmode && type) > - ? int_size_in_bytes (type) : (int) GET_MODE_SIZE (mode); > + if (mode == BLKmode && type) > + size = int_size_in_bytes (type); > + else > + /* No frontends can create types with variable-sized modes, so we > + shouldn't be asked to pass or return them. */ > + size = GET_MODE_SIZE (mode).to_constant (); I presume that the assertion in your comment is checked in the GET_MODE_SIZE (mode).to_constant (); call? If not, is it worth making the assert explicit here? > @@ -11874,8 +11921,9 @@ aarch64_simd_valid_immediate (rtx op, si > unsigned int n_elts; > if (const_vec_duplicate_p (op, &elt)) > n_elts = 1; > - else if (GET_CODE (op) == CONST_VECTOR) > - n_elts = CONST_VECTOR_NUNITS (op); > + else if (GET_CODE (op) == CONST_VECTOR > + && CONST_VECTOR_NUNITS (op).is_constant (&n_elts)) > + ; > else > return false; > A comment in the empty else if case would be useful for clarity. Thanks, James
