On Wed, Nov 27, 2013 at 10:43 AM, Eric Botcazou <ebotca...@adacore.com> wrote: >> I think you are right, this flag is no longer necessary, and removing >> this code path would simplify everything. Therefore I'd like to propose >> to remove the "keep_aligning" parameter of get_inner_reference as >> a split-out patch. >> >> Boot-strapped (with languages=all,ada,go) and >> regression-tested on x86_64-linux-gnu. > > I don't understand how you can commit a patch that changes something only on > strict-alignment platforms and test it only on x86-64. This change *must* be > tested with Ada on a strict-alignment platform, that's the only combination > for which it is exercised. If you cannot do that, then please back it out. > > More generally speaking, it's not acceptable to make cleanup changes like that > in the RTL expander without extreme care, which of course starts with proper > testing. The patch should not have been approved either for that reason.
I'm fine with reverting it for now (you were in CC of the patch submission but silent on it, I asked for the patch to start simplifying the way mems are expanded - ultimately to avoid the recursion and mem-attribute compute by the recursion). We can come back during stage1. get_object_alignment should be able to properly handle this case if you call it on the full reference in the normal_inner_ref: case. All the weird duplicate code on the VIEW_CONVERT_EXPR case should IMHO go. Richard. > -- > Eric Botcazou
