This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Strict volatile bit-fields clean-up, Take 2
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Bernd Edlinger <bernd dot edlinger at hotmail dot de>
- Cc: Eric Botcazou <ebotcazou at adacore dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 3 Dec 2013 12:23:11 +0100
- Subject: Re: [PATCH] Strict volatile bit-fields clean-up, Take 2
- Authentication-results: sourceware.org; auth=none
- References: <DUB122-W2205631605E094F653823BE4D50 at phx dot gbl>
On Tue, Dec 3, 2013 at 12:01 PM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> Hi,
>
> This is my proposal for ulimately getting rid of the nasty store_fixed_bit_field recursion.
>
> IMHO, the root of the recursion trouble is here:
>
> @@ -1007,12 +1013,8 @@ store_fixed_bit_field (rtx op0, unsigned
>
> if (MEM_P (op0))
> {
> mode = GET_MODE (op0);
> if (GET_MODE_BITSIZE (mode) == 0
> || GET_MODE_BITSIZE (mode)> GET_MODE_BITSIZE (word_mode))
> mode = word_mode;
> mode = get_best_mode (bitsize, bitnum, bitregion_start, bitregion_end,
> MEM_ALIGN (op0), mode, MEM_VOLATILE_P (op0));
>
>
> But, because now we always have bitregion_start and bitregion_end to limit
> the access size, it is no longer necessary to restrict the largest mode, that
> get_best_mode may return.
Note that this is not true, as get_bit_range itself may end up giving
up with bitregion_start = bitregion_end = 0. But maybe this is not
what you are saying here? That is, does a
gcc_assert (bitregion_start != 0 || bitregion_end != 0);
before the get_best_mode call work for you?
> This patch is very similar to the previous patch, which split up the extract_fixed_bit_field,
>
> This time, I just split up store_fixed_bit_field and use store_fixed_bit_field_1 to force the
> strict-volatile-bitfield mode it necessary, and let get_best_mode find a mode, that is
> can be used to access the field, which is no longer impacted by the memory context's selected
> mode in this case.
>
> I tried this patch with an ARM-Cross compiler and a large eCos application, to see if anything
> changes in the generated code with this patch, but 2 MB of code stays binary the same,
> that's a good sign.
>
> I added the new Ada test case, and the test case from PR59134, which does no longer re-produce
> after my previous C++ memory model patch, but this "fix" was more or less by chance.
>
>
> Boot-Strap on X86_64-pc-linux-gnu (languages=all,ada,go) and regression-tests
> still running.
>
>
> Ok for trunk (when the tests succeed)?
>
>
> Thanks
> Bernd.