On Sat, Jan 11, 2014 at 12:42 AM, Eric Botcazou <ebotca...@adacore.com> wrote: > [Sorry for dropping the ball here] > >> I think that may_be_unaligned_p is just seriously out-dated ... shouldn't it >> be sth like >> >> get_object_alignment_1 (ref, &align, &bitpos); >> if step * BITS_PER_UNIT + bitpos is misaligned >> ... >> >> or rather all this may_be_unaligned_p stuff should be dropped and IVOPTs >> should finally generate proper [TARGET_]MEM_REFs instead? That is, >> we already handle aliasing fine: >> >> ref = create_mem_ref (&bsi, TREE_TYPE (*use->op_p), &aff, >> reference_alias_ptr_type (*use->op_p), >> iv, base_hint, data->speed); >> >> so just also handle alignment properly by passing down >> get_object_alignment (*use->op_p) and in create_mem_ref_raw >> do at the end do the >> >> if (TYPE_MODE (type) != BLKmode >> && GET_MODE_ALIGNMENT (TYPE_MODE (type)) > align) >> type = build_aligned_type (type, align); >> >> for BLKmode we already look at TYPE_ALIGN and as we do not change >> the access type(?) either the previous code was already wrong or it was >> fine, so there is nothing to do. >> >> So - if you want to give it a try...? > > After a bit of pondering, I'm not really thrilled, as this would mean changing > TARGET_MEM_REF to accept invalid (unaligned) memory references for the target.
AFAIK the expander already handles this if the target can expand it via movmisalign at least. One issue with vectorization is that possibly unaligned vector accesses are not handled/optimized by IVOPTs which is bad. Something to re-visit for 4.10. > But I agree that may_be_unaligned_p is seriously outdated, so the attached > patch entirely rewrites it, fixing the bug in the process. > > Tested on SPARC, SPARC64, IA-64 and ARM, OK for the mainline? OK. Note that this now lets unaligned vector moves slip through as their TYPE_ALIGN (TREE_TYPE (ref)) is properly reflecting this fact, so is anything which dereferences a type with an aligned attribute lowering its alignment. Which of course raises the question what the function is supposed to verify alignment against - given that it is only queried for STRICT_ALIGNMENT targets I would guess it wants to verify against mode alignment (historically at least ...). Not sure how this observation relates to the bug you want to fix though. Still the patch is an improvement and thus ok. Thanks, Richard. > 2014-01-10 Eric Botcazou <ebotca...@adacore.com> > > * builtins.c (get_object_alignment_2): Minor tweak. > * tree-ssa-loop-ivopts.c (may_be_unaligned_p): Rewrite. > > > -- > Eric Botcazou
