This is the mail archive of the
mailing list for the GCC project.
RE: [PATCH] Remove "keep_aligning" from get_inner_reference
- From: Bernd Edlinger <bernd dot edlinger at hotmail dot de>
- To: Eric Botcazou <ebotcazou at adacore dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Richard Biener <richard dot guenther at gmail dot com>, Martin Jambor <mjambor at suse dot cz>, Jakub Jelinek <jakub at redhat dot com>
- Date: Wed, 27 Nov 2013 12:04:07 +0100
- Subject: RE: [PATCH] Remove "keep_aligning" from get_inner_reference
- Authentication-results: sourceware.org; auth=none
- References: <20130910193228 dot GE6732 at virgil dot suse> <CAFiYyc1KcuO+HkqmDCNqehDjzj3hQBHCxKfBzqvRd7vbmyTUCg at mail dot gmail dot com> <DUB122-W4FD979DA755110F3939B4E40F0 at phx dot gbl>,<11089176 dot P1umEoB5Oc at polaris>
On 27 Nov 2013 10:43:59, Eric Botcazou 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
Well, I did that. Apologies for not mentioning that.
> 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.
> Eric Botcazou
The change on the ada interface is actually not critical, because all invocations
of get_inner_reference there used keep_aligning == false, as did the majority of
all other invocations.
What changes with that patch, is that get_inner_reference(...., true) could return
a VIEW_CONVERT_EXPR, which is now obsolete.
If it is causing any trouble, I can revert that change of course.