> The problem with this intermediate inconsistency is that ...
> 
> > That's why only the final result should matter and need be correct.
> 
> ... this can't be ensured anymore.  In some cases the inconsistency
> between the mem attributes and what XEXP(MEM, 0) represents can't be
> repaired by the later bitpos adjustments, or better it can be only be
> repaired by falling back to the conservative side for e.g. the alignment,
> because we don't store enough information in the mem attributes to recover
> what was lost.

Indeed, looking at the history of the code shows that the alignment handling 
via MEM_REF and get_object_alignment is an afterthought.  Originally, it was 
set only in a few specific cases:

  /* We can set the alignment from the type if we are making an object,
     this is an INDIRECT_REF, or if TYPE_ALIGN_OK.  */

and also for DECL_P and CONSTANT_CLASS_P.  That was it, so this was actually 
the alignment of the base and disregarding BITPOS was fine.

> When briefly discussing this yesterday I suggested (without having the
> code in front of me) that it be best to simply set the mem attributes only
> at the very end, after having computed to final MEM address including all
> adjustments, instead of generating wrong mem attributes initially and
> hoping for the adjustments to make it come out right at the end.

Maybe, but for 4.7 (and probably 4.8) I think we should be conservative and 
only fix the problems recently introduced.

So, in the end, I think that we should go either for Richard's initial patch 
in this thread, possibly with a ??? comment along the lines of:

        /* ??? If we haven't computed the alignment yet, compute it now.  Note
            that, if already computed above, the alignment is that of the base
            object of T so it is OK to have disregarded BITPOS above.  However,
            here we are using get_object_alignment_1 which returns the precise
            alignment of T so we need to account for the BITPOS adjustment.  */
        if (!align_computed)

or for Richard's initial patch + the removal of the MEM_REF block (which is 
midway between Richard's initial patch and Richard's last patch).

-- 
Eric Botcazou

Reply via email to