This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR55882
> 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