Hi,
On Thu, 5 Mar 2015 10:00:26, Richard Biener wrote:
>
> On Wed, Mar 4, 2015 at 3:13 PM, Bernd Edlinger
> <bernd.edlin...@hotmail.de> wrote:
>> bounced... again, without html.
>>
>>
>> Hi Richard,
>>
>> while working on another bug in the area of -fstrict-volatile-bitfields
>> I became aware of another example where -fstrict-volatile-bitfields may
>> generate
>> wrong code. This is reproducible on a !STRICT_ALIGNMENT target like x86_64.
>>
>> The problem is that strict_volatile_bitfield_p tries to allow more than
>> necessary
>> if !STRICT_ALIGNMENT. Everything works OK on ARM for instance.
>>
>> If this function returns true, we may later call narrow_bit_field_mem, and
>> the check in strict_volatile_bitfield_p should mirror the logic there:
>> narrow_bit_field_mem just uses GET_MODE_BITSIZE (mode) and does not
>> care about STRICT_ALIGNMENT, and in the end *new_bitnum + bitsize may
>> reach beyond the end of the region. This causes store_fixed_bit_field_1
>> to silently fail to generate correct code.
>
> Hmm, but the comment sounds like if using GET_MODE_ALIGNMENT is
> more correct (even for !strict-alignment) - if mode is SImode and mode
> alignment is 16 (HImode aligned) then we don't need to split the load
> if bitnum is 16 and bitsize is 32.
>
> So maybe narrow_bit_field_mem needs to be fixed as well?
>
I'd rather not touch that function....
In the whole expmed.c the only place where GET_MODE_ALIGNMENT
is used, is in simple_mem_bitfield_p but only if SLOW_UNALIGNED_ACCESS
returns 1, which is only the case on few targets.
Do you know any targets, where GET_MODE_ALIGNMENT may be less than
GET_MODE_BITSIZE?
Maybe one thing is missing from strict_volatile_bitfield_p, I am not sure.
Maybe it should check that MEM_ALIGN (op0)>= GET_MODE_ALIGNMENT (fieldmode)
Because the strict volatile bitfields handling will inevitably try to use
the fieldmode to access the memory.
Or would it be better to say MEM_ALIGN (op0)>= GET_MODE_BITSIZE (fieldmode),
because it is easier to explain when some one asks, when we guarantee the
semantics
of strict volatile bitfield?
Probably there is already something in the logic in expr.c that prevents these
cases,
because otherwise it would be way to easy to find an example for unaligned
accesses
to unaligned memory on STRICT_ALIGNMENT targets.
Ok, what would you think about this variant?
--- expmed.c.jj 2015-01-16 11:20:40.000000000 +0100
+++ expmed.c 2015-03-05 11:50:09.400444416 +0100
@@ -472,9 +472,11 @@ strict_volatile_bitfield_p (rtx op0, uns
return false;
/* Check for cases of unaligned fields that must be split. */
- if (bitnum % BITS_PER_UNIT + bitsize> modesize
- || (STRICT_ALIGNMENT
- && bitnum % GET_MODE_ALIGNMENT (fieldmode) + bitsize> modesize))
+ if (bitnum % modesize + bitsize> modesize)
+ return false;
+
+ /* Check that the memory is sufficiently aligned. */
+ if (MEM_ALIGN (op0) < modesize)
return false;
/* Check for cases where the C++ memory model applies. */
Trying to use an atomic access to a device register is pointless if the
memory context is not aligned to the MODE_BITSIZE, that has nothing
to do with MODE_ALIGNMENT, right?
Thanks
Bernd.
