On Tue, 3 Dec 2013 12:23:11, Richard Biener wrote:
>
> On Tue, Dec 3, 2013 at 12:01 PM, Bernd Edlinger
> <bernd.edlin...@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?
Unfortunately not in Ada, BUT there must be some implicit guarantees
what a store_bit_field(to, bitsize, bitpos, 0, 0, VOIDmode, value) may
overwrite. I always assumed the [bitpos:bitpos+bitsize-1] is just stretched
to the next MEM_ALIGN(to) or WORD_MODE boundary whichever is less?
After all extract_bit_field uses the same bounding region.
The problem is that there are cases where the mode in the CONTEXT is
just too small. And the fall-back must be bullet-proof.
>
>> 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.
