On 05/11/18 12:41, Richard Biener wrote: > On Mon, Nov 5, 2018 at 1:07 PM Andre Vieira (lists) > <andre.simoesdiasvie...@arm.com> wrote: >> >> >> Hi, >> Hi,
Thank you for the quick response! See inline responses below. >> This patch enables targets to describe DR_TARGET_ALIGNMENT as a >> compile-time variable. It does so by turning the variable into a >> 'poly_uint64'. This should not affect the current code-generation for >> any target. >> >> We hope to use this in the near future for SVE using the >> current_vector_size as the preferred target alignment for vectors. In >> fact I have a patch to do just this, but I am still trying to figure out >> whether and when it is beneficial to peel for alignment with a runtime >> misalignment. > > In fact in most cases I have seen the issue is that it's not visible whether > peeling will be able to align _all_ references and doing peeling only to > align some is hardly beneficial. To improve things the vectorizer would > have to version the loop for the case where peeling can reach alignment > for a group of DRs and then vectorize one copy with peeling for alignment > and one copy with unaligned accesses. So I have seen code being peeled for alignment even when it only knows how to align one of a group (only checked 2 or 3) and I think this may still be beneficial in some cases. I am more worried about cases where the number of iterations isn't enough to justify the initial peeling cost or when the loop isn't memory bound, i.e. very arithmetic heavy loops. This is a bigger vectorization problem though, that would require some kind of cost-model. > >> The patch I am working on will change the behavior of >> auto-vectorization for SVE when building vector-length agnostic code for >> targets that benefit from aligned vector loads/stores. The patch will >> result in the generation of a runtime computation of misalignment and >> the construction of a corresponding mask for the first iteration of the >> loop. >> >> I have decided to not offer support for prolog/epilog peeling when the >> target alignment is not compile-time constant, as this didn't seem >> useful, this is why 'vect_do_peeling' returns early if >> DR_TARGET_ALIGNMENT is not constant. >> >> I bootstrapped and tested this on aarch64 and x86 basically >> bootstrapping one target that uses this hook and one that doesn't. >> >> Is this OK for trunk? > > The patch looks good but I wonder wheter it is really necessary at this > point. The goal of this patch is really to enable future work, on it's own it does nothing. I am working on a small target-specific patch to enable this for SVE, but I need to do a bit more analysis and benchmarking to be able to determine whether its beneficial which I will not be able to finish before end of stage 1. That is why I split them up and sent this one upstream to see if I could get the middle-end change in. > > Thanks, > Richard. > >> Cheers, >> Andre >> >> 2018-11-05 Andre Vieira <andre.simoesdiasvie...@arm.com> >> >> * config/aarch64/aarch64.c >> (aarch64_vectorize_preferred_vector_alignment): >> Change return type to poly_uint64. >> (aarch64_simd_vector_alignment_reachable): Adapt to preferred vector >> alignment being a poly int. >> * doc/tm.texi (TARGET_VECTORIZE_PREFERRED_VECTOR_ALIGNMENT): Change >> return >> type to poly_uint64. >> * target.def (default_preferred_vector_alignment): Likewise. >> * targhooks.c (default_preferred_vector_alignment): Likewise. >> * targhooks.h (default_preferred_vector_alignment): Likewise. >> * tree-vect-data-refs.c >> (vect_calculate_target_alignment): Likewise. >> (vect_compute_data_ref_alignment): Adapt to vector alignment >> being a poly int. >> (vect_update_misalignment_for_peel): Likewise. >> (vect_enhance_data_refs_alignment): Likewise. >> (vect_find_same_alignment_drs): Likewise. >> (vect_duplicate_ssa_name_ptr_info): Likewise. >> (vect_setup_realignment): Likewise. >> (vect_can_force_dr_alignment_p): Change alignment parameter type to >> poly_uint64. >> * tree-vect-loop-manip.c (get_misalign_in_elems): Learn to construct >> a mask >> with a compile time variable vector alignment. >> (vect_gen_prolog_loop_niters): Adapt to vector alignment being a >> poly int. >> (vect_do_peeling): Exit early if vector alignment is not constant. >> * tree-vect-stmts.c (ensure_base_align): Adapt to vector alignment >> being a >> poly int. >> (vectorizable_store): Likewise. >> (vectorizable_load): Likweise. >> * tree-vectorizer.h (struct dr_vec_info): Make target_alignment >> field a >> poly_uint64. >> (vect_known_alignment_in_bytes): Adapt to vector alignment being a >> poly >> int. >> (vect_can_force_dr_alignment_p): Change alignment parameter type to >> poly_uint64.
