On Mon, Jun 26, 2017 at 12:25 PM, Richard Biener <richard.guent...@gmail.com> wrote: > On Fri, Jun 23, 2017 at 2:05 PM, Richard Sandiford > <richard.sandif...@linaro.org> wrote: >> Richard Biener <richard.guent...@gmail.com> writes: >>> On Thu, Jun 22, 2017 at 1:30 PM, Richard Sandiford >>> <richard.sandif...@linaro.org> wrote: >>>> The test case triggered this assert in vect_update_misalignment_for_peel: >>>> >>>> gcc_assert (DR_MISALIGNMENT (dr) / dr_size == >>>> DR_MISALIGNMENT (dr_peel) / dr_peel_size); >>>> >>>> We knew that the two DRs had the same misalignment at runtime, but when >>>> considered in isolation, one data reference guaranteed a higher >>>> compile-time >>>> base alignment than the other. >>>> >>>> In the test case this looks like a missed opportunity. Both references >>>> are unconditional, so it should be possible to use the highest of the >>>> available base alignment guarantees when analyzing each reference. >>>> The patch does this. >>>> >>>> However, as the comment in the patch says, the base alignment guarantees >>>> provided by a conditional reference only apply if the reference occurs >>>> at least once. In this case it would be legitimate for two references >>>> to have the same runtime misalignment and for one reference to provide a >>>> stronger compile-time guarantee than the other about what the misalignment >>>> actually is. The patch therefore relaxes the assert to handle that case. >>> >>> Hmm, but you don't actually check whether a reference occurs only >>> conditional, >>> do you? You just seem to say that for masked loads/stores the reference >>> is conditional (I believe that's not true). But for a loop like >>> >>> for (;;) >>> if (a[i]) >>> sum += b[j]; >>> >>> you still assume b[j] executes unconditionally? >> >> Maybe the documentation isn't clear enough, but DR_IS_CONDITIONAL >> was supposed to mean "even if the containing statement executes >> and runs to completion, the reference might not actually occur". >> The example above isn't conditional in that sense because the >> reference to b[j] does occur if the store is reached and completes. >> >> Masked loads and stores are conditional in that sense though. >> The reference only occurs if the mask is nonzero; the memory >> isn't touched otherwise. The functions are used to if-convert >> things like: >> >> for (...) >> a[i] = b[i] ? c[i] : d[i]; >> >> where there's no guarantee that it's safe to access c[i] when !b[i] >> (or d[i] when b[i]). No reference occurs for an all-false mask. > > But as you touch generic data-ref code here you should apply more > sensible semantics to DR_IS_CONDITIONAL than just marking > masked loads/stores but not DRs occuring inside BBs only executed > conditionally ... > >>> The vectorizer of course only sees unconditionally executed stmts. >>> >>> So - I'd simply not add this DR_IS_CONDITIONAL. Did you run into >>> any real-world (testsuite) issues without this? >> >> Dropping DR_IS_CONDITIONAL would cause us to make invalid alignment >> assumptions in silly corner cases. I could add a scan test for it, >> for targets with masked loads and stores. It wouldn't trigger >> an execution failure though because we assume that targets with >> masked loads and stores allow unaligned accesses: >> >> /* For now assume all conditional loads/stores support unaligned >> access without any special code. */ >> if (is_gimple_call (stmt) >> && gimple_call_internal_p (stmt) >> && (gimple_call_internal_fn (stmt) == IFN_MASK_LOAD >> || gimple_call_internal_fn (stmt) == IFN_MASK_STORE)) >> return dr_unaligned_supported; >> >> So the worst that would happen is that we'd supposedly peel for >> alignment, but actually misalign everything instead, and so make >> things slower rather than quicker. >> >>> Note that the assert is to prevent bogus information. Iff we aligned >>> DR with base alignment 8 and misalign 3 then if another same-align >>> DR has base alignment 16 we can't simply zero its DR_MISALIGNMENT >>> as it still can be 8 after aligning DR. >>> >>> So I think it's wrong to put DRs with differing base-alignment into >>> the same-align-refs chain, those should get their DR_MISALIGNMENT >>> updated independenlty after peeling. >> >> DR_MISALIGNMENT is relative to the vector alignment rather than >> the base alignment though. So: > > We seem to use it that way, yes (looking at set_ptr_info_alignment > uses). So why not fix the assert then by capping the alignment/misalignment > we compute at this value as well? (and document this in the header > around DR_MISALIGNMENT) > > Ideally we'd do alignment analysis independent of the vector size > though (for those stupid targets with multiple vector sizes to consider...). > >> a) when looking for references *A1 and *A2 with the same alignment, >> we simply have to prove that A1 % vecalign == A2 % vecalign. >> This doesn't require any knowledge about the base alignment. >> If we break the addresses down as: >> >> A1 = BASE1 + REST1, REST1 = INIT1 + OFFSET1 + X * STEP1 >> A2 = BASE2 + REST2, REST2 = INIT2 + OFFSET2 + X * STEP2 >> >> and can prove that BASE1 == BASE2, the alignment of that base >> isn't important. We simply need to prove that REST1 % vecalign >> == REST2 % vecalign for all X. >> >> b) In the assert, we've peeled the loop so that DR_PEEL is guaranteed >> to be vector-aligned. If DR_PEEL is A1 in the example above, we have >> A1 % vecalign == 0, so A2 % vecalign must be 0 too. This again doesn't >> rely on the base alignment being known. >> >> What a high base alignment for DR_PEEL gives us is the ability to know >> at compile how many iterations need to be peeled to make DR_PEEL aligned. >> But the points above apply regardless of whether we know that value at >> compile time or not. >> >> In examples like the test case, we would have known at compile time that >> VF-1 iterations would need to be peeled if we'd picked the store as the >> DR_PEEL, but would have treated the number of peels as variable if we'd >> picked the load. The value calculated at runtime would still have been >> VF-1, it's just that the code wouldn't have been as efficient. >> >> One of the benefits of pooling the alignments for unconditional references >> is that it no longer matters which DR we pick: the number of peels will >> be a compile-time constant both ways. >> >> Thanks, >> Richard >> >>> I'd rather not mix fixing this with the improvement to eventuall use a >>> larger align for the other DR if possible. > > ^^^ > > So can you fix the ICE with capping base alignment / DR_MISALIGNMENT?
Oh, and as you are touching general tree-data-refs.[ch] on my wishlist is still "properly" computing the base alignment in tree-data-refs.[ch] itself rather than trying to undo analyze_innermost in the vectorizer and 2nd-guessing the alignment. Basically make alignment/misalignment first-class citizens. Richard. > Richard. > >>> Thanks, >>> Richard. >>> >>>> Tested on powerpc64-linux-gnu, aarch64-linux-gnu and x86_64-linux-gnu. >>>> OK to instal? >>>> >>>> Richard >>>> >>>> >>>> 2017-06-22 Richard Sandiford <richard.sandif...@linaro.org> >>>> >>>> gcc/ >>>> PR tree-optimization/81136 >>>> * tree-vectorizer.h: Include tree-hash-traits.h. >>>> (vec_base_alignments): New typedef. >>>> (vec_info): Add a base_alignments field. >>>> (vect_compute_base_alignments: Declare. >>>> * tree-data-ref.h (data_reference): Add an is_conditional field. >>>> (DR_IS_CONDITIONAL): New macro. >>>> (create_data_ref): Add an is_conditional argument. >>>> * tree-data-ref.c (create_data_ref): Likewise. Use it to >>>> initialize >>>> the is_conditional field. >>>> (data_ref_loc): Add an is_conditional field. >>>> (get_references_in_stmt): Set the is_conditional field. >>>> (find_data_references_in_stmt): Update call to create_data_ref. >>>> (graphite_find_data_references_in_stmt): Likewise. >>>> * tree-ssa-loop-prefetch.c (determine_loop_nest_reuse): Likewise. >>>> * tree-vect-data-refs.c (vect_analyze_data_refs): Likewise. >>>> (vect_get_base_address): New function. >>>> (vect_compute_base_alignments): Likewise. >>>> (vect_compute_base_alignment): Likewise, split out from... >>>> (vect_compute_data_ref_alignment): ...here. Use precomputed >>>> base alignments. Only compute a new base alignment here if the >>>> reference is conditional. >>>> (vect_update_misalignment_for_peel): Allow the compile-time >>>> DR_MISALIGNMENTs of two references with the same runtime alignment >>>> to be different if one of the references is conditional. >>>> (vect_find_same_alignment_drs): Compare base addresses instead >>>> of base objects. >>>> (vect_compute_data_ref_alignment): Call >>>> vect_compute_base_alignments. >>>> * tree-vect-slp.c (vect_slp_analyze_bb_1): Likewise. >>>> (new_bb_vec_info): Initialize base_alignments. >>>> * tree-vect-loop.c (new_loop_vec_info): Likewise. >>>> * tree-vectorizer.c (vect_destroy_datarefs): Release >>>> base_alignments. >>>> >>>> gcc/testsuite/ >>>> PR tree-optimization/81136 >>>> * gcc.dg/vect/pr81136.c: New test. >>>> >>>> Index: gcc/tree-vectorizer.h >>>> =================================================================== >>>> --- gcc/tree-vectorizer.h 2017-06-08 08:51:43.347264181 +0100 >>>> +++ gcc/tree-vectorizer.h 2017-06-22 12:23:21.288421018 +0100 >>>> @@ -22,6 +22,7 @@ Software Foundation; either version 3, o >>>> #define GCC_TREE_VECTORIZER_H >>>> >>>> #include "tree-data-ref.h" >>>> +#include "tree-hash-traits.h" >>>> #include "target.h" >>>> >>>> /* Used for naming of new temporaries. */ >>>> @@ -84,6 +85,10 @@ struct stmt_info_for_cost { >>>> >>>> typedef vec<stmt_info_for_cost> stmt_vector_for_cost; >>>> >>>> +/* Maps base addresses to the largest alignment that we've been able >>>> + to calculate for them. */ >>>> +typedef hash_map<tree_operand_hash, unsigned int> vec_base_alignments; >>>> + >>>> /************************************************************************ >>>> SLP >>>> ************************************************************************/ >>>> @@ -156,6 +161,10 @@ struct vec_info { >>>> /* All data references. */ >>>> vec<data_reference_p> datarefs; >>>> >>>> + /* Maps the base addresses of all data references in DATAREFS to the >>>> + largest alignment that we've been able to calculate for them. */ >>>> + vec_base_alignments base_alignments; >>>> + >>>> /* All data dependences. */ >>>> vec<ddr_p> ddrs; >>>> >>>> @@ -1117,6 +1126,7 @@ extern bool vect_prune_runtime_alias_tes >>>> extern bool vect_check_gather_scatter (gimple *, loop_vec_info, >>>> gather_scatter_info *); >>>> extern bool vect_analyze_data_refs (vec_info *, int *); >>>> +extern void vect_compute_base_alignments (vec_info *); >>>> extern tree vect_create_data_ref_ptr (gimple *, tree, struct loop *, tree, >>>> tree *, gimple_stmt_iterator *, >>>> gimple **, bool, bool *, >>>> Index: gcc/tree-data-ref.h >>>> =================================================================== >>>> --- gcc/tree-data-ref.h 2017-06-08 08:51:43.349263895 +0100 >>>> +++ gcc/tree-data-ref.h 2017-06-22 12:23:21.285421180 +0100 >>>> @@ -119,6 +119,10 @@ struct data_reference >>>> /* True when the data reference is in RHS of a stmt. */ >>>> bool is_read; >>>> >>>> + /* True when the data reference is conditional, i.e. if it might not >>>> + occur even when the statement runs to completion. */ >>>> + bool is_conditional; >>>> + >>>> /* Behavior of the memory reference in the innermost loop. */ >>>> struct innermost_loop_behavior innermost; >>>> >>>> @@ -138,6 +142,7 @@ #define DR_ACCESS_FN(DR, I) DR_AC >>>> #define DR_NUM_DIMENSIONS(DR) DR_ACCESS_FNS (DR).length () >>>> #define DR_IS_READ(DR) (DR)->is_read >>>> #define DR_IS_WRITE(DR) (!DR_IS_READ (DR)) >>>> +#define DR_IS_CONDITIONAL(DR) (DR)->is_conditional >>>> #define DR_BASE_ADDRESS(DR) (DR)->innermost.base_address >>>> #define DR_OFFSET(DR) (DR)->innermost.offset >>>> #define DR_INIT(DR) (DR)->innermost.init >>>> @@ -350,7 +355,8 @@ extern bool graphite_find_data_reference >>>> vec<data_reference_p> >>>> *); >>>> tree find_data_references_in_loop (struct loop *, vec<data_reference_p> >>>> *); >>>> bool loop_nest_has_data_refs (loop_p loop); >>>> -struct data_reference *create_data_ref (loop_p, loop_p, tree, gimple *, >>>> bool); >>>> +struct data_reference *create_data_ref (loop_p, loop_p, tree, gimple *, >>>> bool, >>>> + bool); >>>> extern bool find_loop_nest (struct loop *, vec<loop_p> *); >>>> extern struct data_dependence_relation >>>> *initialize_data_dependence_relation >>>> (struct data_reference *, struct data_reference *, vec<loop_p>); >>>> Index: gcc/tree-data-ref.c >>>> =================================================================== >>>> --- gcc/tree-data-ref.c 2017-06-08 08:51:43.349263895 +0100 >>>> +++ gcc/tree-data-ref.c 2017-06-22 12:23:21.284421233 +0100 >>>> @@ -1053,15 +1053,18 @@ free_data_ref (data_reference_p dr) >>>> free (dr); >>>> } >>>> >>>> -/* Analyzes memory reference MEMREF accessed in STMT. The reference >>>> - is read if IS_READ is true, write otherwise. Returns the >>>> - data_reference description of MEMREF. NEST is the outermost loop >>>> - in which the reference should be instantiated, LOOP is the loop in >>>> - which the data reference should be analyzed. */ >>>> +/* Analyze memory reference MEMREF, which is accessed in STMT. The >>>> reference >>>> + is a read if IS_READ is true, otherwise it is a write. IS_CONDITIONAL >>>> + indicates that the reference is conditional, i.e. that it might not >>>> + occur every time that STMT runs to completion. >>>> + >>>> + Return the data_reference description of MEMREF. NEST is the outermost >>>> + loop in which the reference should be instantiated, LOOP is the loop >>>> + in which the data reference should be analyzed. */ >>>> >>>> struct data_reference * >>>> create_data_ref (loop_p nest, loop_p loop, tree memref, gimple *stmt, >>>> - bool is_read) >>>> + bool is_read, bool is_conditional) >>>> { >>>> struct data_reference *dr; >>>> >>>> @@ -1076,6 +1079,7 @@ create_data_ref (loop_p nest, loop_p loo >>>> DR_STMT (dr) = stmt; >>>> DR_REF (dr) = memref; >>>> DR_IS_READ (dr) = is_read; >>>> + DR_IS_CONDITIONAL (dr) = is_conditional; >>>> >>>> dr_analyze_innermost (dr, nest); >>>> dr_analyze_indices (dr, nest, loop); >>>> @@ -4446,6 +4450,10 @@ struct data_ref_loc >>>> >>>> /* True if the memory reference is read. */ >>>> bool is_read; >>>> + >>>> + /* True if the data reference is conditional, i.e. if it might not >>>> + occur even when the statement runs to completion. */ >>>> + bool is_conditional; >>>> }; >>>> >>>> >>>> @@ -4512,6 +4520,7 @@ get_references_in_stmt (gimple *stmt, ve >>>> { >>>> ref.ref = op1; >>>> ref.is_read = true; >>>> + ref.is_conditional = false; >>>> references->safe_push (ref); >>>> } >>>> } >>>> @@ -4539,6 +4548,7 @@ get_references_in_stmt (gimple *stmt, ve >>>> type = TREE_TYPE (gimple_call_arg (stmt, 3)); >>>> if (TYPE_ALIGN (type) != align) >>>> type = build_aligned_type (type, align); >>>> + ref.is_conditional = true; >>>> ref.ref = fold_build2 (MEM_REF, type, gimple_call_arg (stmt, >>>> 0), >>>> ptr); >>>> references->safe_push (ref); >>>> @@ -4558,6 +4568,7 @@ get_references_in_stmt (gimple *stmt, ve >>>> { >>>> ref.ref = op1; >>>> ref.is_read = true; >>>> + ref.is_conditional = false; >>>> references->safe_push (ref); >>>> } >>>> } >>>> @@ -4571,6 +4582,7 @@ get_references_in_stmt (gimple *stmt, ve >>>> { >>>> ref.ref = op0; >>>> ref.is_read = false; >>>> + ref.is_conditional = false; >>>> references->safe_push (ref); >>>> } >>>> return clobbers_memory; >>>> @@ -4635,8 +4647,8 @@ find_data_references_in_stmt (struct loo >>>> >>>> FOR_EACH_VEC_ELT (references, i, ref) >>>> { >>>> - dr = create_data_ref (nest, loop_containing_stmt (stmt), >>>> - ref->ref, stmt, ref->is_read); >>>> + dr = create_data_ref (nest, loop_containing_stmt (stmt), ref->ref, >>>> + stmt, ref->is_read, ref->is_conditional); >>>> gcc_assert (dr != NULL); >>>> datarefs->safe_push (dr); >>>> } >>>> @@ -4665,7 +4677,8 @@ graphite_find_data_references_in_stmt (l >>>> >>>> FOR_EACH_VEC_ELT (references, i, ref) >>>> { >>>> - dr = create_data_ref (nest, loop, ref->ref, stmt, ref->is_read); >>>> + dr = create_data_ref (nest, loop, ref->ref, stmt, ref->is_read, >>>> + ref->is_conditional); >>>> gcc_assert (dr != NULL); >>>> datarefs->safe_push (dr); >>>> } >>>> Index: gcc/tree-ssa-loop-prefetch.c >>>> =================================================================== >>>> --- gcc/tree-ssa-loop-prefetch.c 2017-06-07 21:58:55.928557601 +0100 >>>> +++ gcc/tree-ssa-loop-prefetch.c 2017-06-22 12:23:21.285421180 +0100 >>>> @@ -1633,7 +1633,7 @@ determine_loop_nest_reuse (struct loop * >>>> for (ref = gr->refs; ref; ref = ref->next) >>>> { >>>> dr = create_data_ref (nest, loop_containing_stmt (ref->stmt), >>>> - ref->mem, ref->stmt, !ref->write_p); >>>> + ref->mem, ref->stmt, !ref->write_p, false); >>>> >>>> if (dr) >>>> { >>>> Index: gcc/tree-vect-data-refs.c >>>> =================================================================== >>>> --- gcc/tree-vect-data-refs.c 2017-06-08 08:51:43.350263752 +0100 >>>> +++ gcc/tree-vect-data-refs.c 2017-06-22 12:23:21.286421126 +0100 >>>> @@ -646,6 +646,102 @@ vect_slp_analyze_instance_dependence (sl >>>> return res; >>>> } >>>> >>>> +/* If DR is nested in a loop that is being vectorized, return the base >>>> + address in the context of the vectorized loop (rather than the >>>> + nested loop). Otherwise return the base address in the context >>>> + of the containing statement. */ >>>> + >>>> +static tree >>>> +vect_get_base_address (data_reference *dr) >>>> +{ >>>> + gimple *stmt = DR_STMT (dr); >>>> + stmt_vec_info stmt_info = vinfo_for_stmt (stmt); >>>> + loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_info); >>>> + struct loop *loop = loop_vinfo != NULL ? LOOP_VINFO_LOOP (loop_vinfo) : >>>> NULL; >>>> + if (loop && nested_in_vect_loop_p (loop, stmt)) >>>> + return STMT_VINFO_DR_BASE_ADDRESS (stmt_info); >>>> + else >>>> + return DR_BASE_ADDRESS (dr); >>>> +} >>>> + >>>> +/* Compute and return the alignment of base address BASE_ADDR in DR. */ >>>> + >>>> +static unsigned int >>>> +vect_compute_base_alignment (data_reference *dr, tree base_addr) >>>> +{ >>>> + /* To look at the alignment of the base we have to preserve an inner >>>> + MEM_REF as that carries the alignment information of the actual >>>> + access. */ >>>> + tree base = DR_REF (dr); >>>> + while (handled_component_p (base)) >>>> + base = TREE_OPERAND (base, 0); >>>> + unsigned int base_alignment = 0; >>>> + unsigned HOST_WIDE_INT base_bitpos; >>>> + get_object_alignment_1 (base, &base_alignment, &base_bitpos); >>>> + >>>> + /* As data-ref analysis strips the MEM_REF down to its base operand >>>> + to form DR_BASE_ADDRESS and adds the offset to DR_INIT we have to >>>> + adjust things to make base_alignment valid as the alignment of >>>> + DR_BASE_ADDRESS. */ >>>> + if (TREE_CODE (base) == MEM_REF) >>>> + { >>>> + /* Note all this only works if DR_BASE_ADDRESS is the same as >>>> + MEM_REF operand zero, otherwise DR/SCEV analysis might have >>>> factored >>>> + in other offsets. We need to rework DR to compute the alingment >>>> + of DR_BASE_ADDRESS as long as all information is still available. >>>> */ >>>> + if (operand_equal_p (TREE_OPERAND (base, 0), base_addr, 0)) >>>> + { >>>> + base_bitpos -= mem_ref_offset (base).to_short_addr () * >>>> BITS_PER_UNIT; >>>> + base_bitpos &= (base_alignment - 1); >>>> + } >>>> + else >>>> + base_bitpos = BITS_PER_UNIT; >>>> + } >>>> + if (base_bitpos != 0) >>>> + base_alignment = base_bitpos & -base_bitpos; >>>> + >>>> + /* Also look at the alignment of the base address DR analysis >>>> + computed. */ >>>> + unsigned int base_addr_alignment = get_pointer_alignment (base_addr); >>>> + if (base_addr_alignment > base_alignment) >>>> + base_alignment = base_addr_alignment; >>>> + >>>> + return base_alignment; >>>> +} >>>> + >>>> +/* Compute alignments for the base addresses of all datarefs in VINFO. */ >>>> + >>>> +void >>>> +vect_compute_base_alignments (vec_info *vinfo) >>>> +{ >>>> + /* If the region we're going to vectorize is reached, all unconditional >>>> + data references occur at least once. We can therefore pool the base >>>> + alignment guarantees from each unconditional reference. */ >>>> + data_reference *dr; >>>> + unsigned int i; >>>> + FOR_EACH_VEC_ELT (vinfo->datarefs, i, dr) >>>> + if (!DR_IS_CONDITIONAL (dr)) >>>> + { >>>> + tree base_addr = vect_get_base_address (dr); >>>> + unsigned int alignment = vect_compute_base_alignment (dr, >>>> base_addr); >>>> + bool existed; >>>> + unsigned int &entry >>>> + = vinfo->base_alignments.get_or_insert (base_addr, &existed); >>>> + if (!existed || entry < alignment) >>>> + { >>>> + entry = alignment; >>>> + if (dump_enabled_p ()) >>>> + { >>>> + dump_printf_loc (MSG_NOTE, vect_location, >>>> + "setting base alignment for "); >>>> + dump_generic_expr (MSG_NOTE, TDF_SLIM, base_addr); >>>> + dump_printf (MSG_NOTE, " to %d, based on ", alignment); >>>> + dump_gimple_stmt (MSG_NOTE, TDF_SLIM, DR_STMT (dr), 0); >>>> + } >>>> + } >>>> + } >>>> +} >>>> + >>>> /* Function vect_compute_data_ref_alignment >>>> >>>> Compute the misalignment of the data reference DR. >>>> @@ -663,6 +759,7 @@ vect_compute_data_ref_alignment (struct >>>> { >>>> gimple *stmt = DR_STMT (dr); >>>> stmt_vec_info stmt_info = vinfo_for_stmt (stmt); >>>> + vec_base_alignments *base_alignments = >>>> &stmt_info->vinfo->base_alignments; >>>> loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_info); >>>> struct loop *loop = NULL; >>>> tree ref = DR_REF (dr); >>>> @@ -699,6 +796,8 @@ vect_compute_data_ref_alignment (struct >>>> { >>>> tree step = DR_STEP (dr); >>>> >>>> + base_addr = STMT_VINFO_DR_BASE_ADDRESS (stmt_info); >>>> + aligned_to = STMT_VINFO_DR_ALIGNED_TO (stmt_info); >>>> if (tree_fits_shwi_p (step) >>>> && tree_to_shwi (step) % GET_MODE_SIZE (TYPE_MODE (vectype)) == >>>> 0) >>>> { >>>> @@ -706,8 +805,6 @@ vect_compute_data_ref_alignment (struct >>>> dump_printf_loc (MSG_NOTE, vect_location, >>>> "inner step divides the vector-size.\n"); >>>> misalign = STMT_VINFO_DR_INIT (stmt_info); >>>> - aligned_to = STMT_VINFO_DR_ALIGNED_TO (stmt_info); >>>> - base_addr = STMT_VINFO_DR_BASE_ADDRESS (stmt_info); >>>> } >>>> else >>>> { >>>> @@ -738,39 +835,15 @@ vect_compute_data_ref_alignment (struct >>>> } >>>> } >>>> >>>> - /* To look at alignment of the base we have to preserve an inner MEM_REF >>>> - as that carries alignment information of the actual access. */ >>>> - base = ref; >>>> - while (handled_component_p (base)) >>>> - base = TREE_OPERAND (base, 0); >>>> + /* Calculate the maximum of the pooled base address alignment and the >>>> + alignment that we can compute for DR itself. The latter should >>>> + already be included in the former for unconditional references. */ >>>> unsigned int base_alignment = 0; >>>> - unsigned HOST_WIDE_INT base_bitpos; >>>> - get_object_alignment_1 (base, &base_alignment, &base_bitpos); >>>> - /* As data-ref analysis strips the MEM_REF down to its base operand >>>> - to form DR_BASE_ADDRESS and adds the offset to DR_INIT we have to >>>> - adjust things to make base_alignment valid as the alignment of >>>> - DR_BASE_ADDRESS. */ >>>> - if (TREE_CODE (base) == MEM_REF) >>>> - { >>>> - /* Note all this only works if DR_BASE_ADDRESS is the same as >>>> - MEM_REF operand zero, otherwise DR/SCEV analysis might have >>>> factored >>>> - in other offsets. We need to rework DR to compute the alingment >>>> - of DR_BASE_ADDRESS as long as all information is still available. >>>> */ >>>> - if (operand_equal_p (TREE_OPERAND (base, 0), base_addr, 0)) >>>> - { >>>> - base_bitpos -= mem_ref_offset (base).to_short_addr () * >>>> BITS_PER_UNIT; >>>> - base_bitpos &= (base_alignment - 1); >>>> - } >>>> - else >>>> - base_bitpos = BITS_PER_UNIT; >>>> - } >>>> - if (base_bitpos != 0) >>>> - base_alignment = base_bitpos & -base_bitpos; >>>> - /* Also look at the alignment of the base address DR analysis >>>> - computed. */ >>>> - unsigned int base_addr_alignment = get_pointer_alignment (base_addr); >>>> - if (base_addr_alignment > base_alignment) >>>> - base_alignment = base_addr_alignment; >>>> + if (DR_IS_CONDITIONAL (dr)) >>>> + base_alignment = vect_compute_base_alignment (dr, base_addr); >>>> + if (unsigned int *entry = base_alignments->get (base_addr)) >>>> + base_alignment = MAX (base_alignment, *entry); >>>> + gcc_assert (base_alignment != 0); >>>> >>>> if (base_alignment >= TYPE_ALIGN (TREE_TYPE (vectype))) >>>> DR_VECT_AUX (dr)->base_element_aligned = true; >>>> @@ -906,8 +979,29 @@ vect_update_misalignment_for_peel (struc >>>> { >>>> if (current_dr != dr) >>>> continue; >>>> - gcc_assert (DR_MISALIGNMENT (dr) / dr_size == >>>> - DR_MISALIGNMENT (dr_peel) / dr_peel_size); >>>> + /* Any alignment guarantees provided by a reference only apply if >>>> + the reference actually occurs. For example, in: >>>> + >>>> + struct s __attribute__((aligned(32))) { >>>> + int misaligner; >>>> + int array[N]; >>>> + }; >>>> + >>>> + int *ptr; >>>> + for (int i = 0; i < n; ++i) >>>> + ptr[i] = c[i] ? ((struct s *) (ptr - 1))->array[i] : 0; >>>> + >>>> + we can only assume that ptr is part of a struct s if at least one >>>> + c[i] is true. This in turn means that we have a higher base >>>> + alignment guarantee for the read from ptr (if it occurs) than for >>>> + the write to ptr, and we cannot unconditionally carry the former >>>> + over to the latter. We still know that the two address values >>>> + have the same misalignment, so if peeling has forced one of them >>>> + to be aligned, the other must be too. */ >>>> + gcc_assert (DR_IS_CONDITIONAL (dr_peel) >>>> + || DR_IS_CONDITIONAL (dr) >>>> + || (DR_MISALIGNMENT (dr) / dr_size >>>> + == DR_MISALIGNMENT (dr_peel) / dr_peel_size)); >>>> SET_DR_MISALIGNMENT (dr, 0); >>>> return; >>>> } >>>> @@ -2117,8 +2211,7 @@ vect_find_same_alignment_drs (struct dat >>>> if (dra == drb) >>>> return; >>>> >>>> - if (!operand_equal_p (DR_BASE_OBJECT (dra), DR_BASE_OBJECT (drb), >>>> - OEP_ADDRESS_OF) >>>> + if (!operand_equal_p (DR_BASE_ADDRESS (dra), DR_BASE_ADDRESS (drb), 0) >>>> || !operand_equal_p (DR_OFFSET (dra), DR_OFFSET (drb), 0) >>>> || !operand_equal_p (DR_STEP (dra), DR_STEP (drb), 0)) >>>> return; >>>> @@ -2176,6 +2269,7 @@ vect_analyze_data_refs_alignment (loop_v >>>> vec<data_reference_p> datarefs = vinfo->datarefs; >>>> struct data_reference *dr; >>>> >>>> + vect_compute_base_alignments (vinfo); >>>> FOR_EACH_VEC_ELT (datarefs, i, dr) >>>> { >>>> stmt_vec_info stmt_info = vinfo_for_stmt (DR_STMT (dr)); >>>> @@ -3374,7 +3468,8 @@ vect_analyze_data_refs (vec_info *vinfo, >>>> { >>>> struct data_reference *newdr >>>> = create_data_ref (NULL, loop_containing_stmt (stmt), >>>> - DR_REF (dr), stmt, maybe_scatter ? >>>> false : true); >>>> + DR_REF (dr), stmt, !maybe_scatter, >>>> + DR_IS_CONDITIONAL (dr)); >>>> gcc_assert (newdr != NULL && DR_REF (newdr)); >>>> if (DR_BASE_ADDRESS (newdr) >>>> && DR_OFFSET (newdr) >>>> Index: gcc/tree-vect-slp.c >>>> =================================================================== >>>> --- gcc/tree-vect-slp.c 2017-06-07 21:58:56.336475882 +0100 >>>> +++ gcc/tree-vect-slp.c 2017-06-22 12:23:21.288421018 +0100 >>>> @@ -2367,6 +2367,7 @@ new_bb_vec_info (gimple_stmt_iterator re >>>> gimple_stmt_iterator gsi; >>>> >>>> res = (bb_vec_info) xcalloc (1, sizeof (struct _bb_vec_info)); >>>> + new (&res->base_alignments) vec_base_alignments (); >>>> res->kind = vec_info::bb; >>>> BB_VINFO_BB (res) = bb; >>>> res->region_begin = region_begin; >>>> @@ -2741,6 +2742,8 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera >>>> return NULL; >>>> } >>>> >>>> + vect_compute_base_alignments (bb_vinfo); >>>> + >>>> /* Analyze and verify the alignment of data references and the >>>> dependence in the SLP instances. */ >>>> for (i = 0; BB_VINFO_SLP_INSTANCES (bb_vinfo).iterate (i, &instance); ) >>>> Index: gcc/tree-vect-loop.c >>>> =================================================================== >>>> --- gcc/tree-vect-loop.c 2017-06-22 12:22:57.734313143 +0100 >>>> +++ gcc/tree-vect-loop.c 2017-06-22 12:23:21.287421072 +0100 >>>> @@ -1157,6 +1157,7 @@ new_loop_vec_info (struct loop *loop) >>>> LOOP_VINFO_VECT_FACTOR (res) = 0; >>>> LOOP_VINFO_LOOP_NEST (res) = vNULL; >>>> LOOP_VINFO_DATAREFS (res) = vNULL; >>>> + new (&res->base_alignments) vec_base_alignments (); >>>> LOOP_VINFO_DDRS (res) = vNULL; >>>> LOOP_VINFO_UNALIGNED_DR (res) = NULL; >>>> LOOP_VINFO_MAY_MISALIGN_STMTS (res) = vNULL; >>>> Index: gcc/tree-vectorizer.c >>>> =================================================================== >>>> --- gcc/tree-vectorizer.c 2017-06-22 12:22:57.732313220 +0100 >>>> +++ gcc/tree-vectorizer.c 2017-06-22 12:23:21.288421018 +0100 >>>> @@ -370,6 +370,8 @@ vect_destroy_datarefs (vec_info *vinfo) >>>> } >>>> >>>> free_data_refs (vinfo->datarefs); >>>> + >>>> + vinfo->base_alignments.~vec_base_alignments (); >>>> } >>>> >>>> /* A helper function to free scev and LOOP niter information, as well as >>>> Index: gcc/testsuite/gcc.dg/vect/pr81136.c >>>> =================================================================== >>>> --- /dev/null 2017-06-22 07:43:14.805493307 +0100 >>>> +++ gcc/testsuite/gcc.dg/vect/pr81136.c 2017-06-22 12:23:21.283421287 +0100 >>>> @@ -0,0 +1,16 @@ >>>> +/* { dg-do compile } */ >>>> + >>>> +struct __attribute__((aligned (32))) >>>> +{ >>>> + char misaligner; >>>> + int foo[100]; >>>> + int bar[100]; >>>> +} *a; >>>> + >>>> +void >>>> +fn1 (int n) >>>> +{ >>>> + int *b = a->foo; >>>> + for (int i = 0; i < n; i++) >>>> + a->bar[i] = b[i]; >>>> +}