On Tue, Sep 17, 2013 at 12:00 PM, Richard Biener
<[email protected]> wrote:
> On Sun, Sep 15, 2013 at 6:55 PM, Bernd Edlinger
> <[email protected]> wrote:
>> Hello Richard,
>>
>> attached is my second attempt at fixing PR 57748. This time the movmisalign
>> path is completely removed and a similar bug in the read handling of
>> misaligned
>> structures with a non-BLKmode is fixed too. There are several new test cases
>> for the
>> different possible failure modes.
>>
>> This patch was boot-strapped and regression tested on
>> x86_64-unknown-linux-gnu
>> and i686-pc-linux-gnu.
>>
>> Additionally I generated eCos and an eCos-application (on ARMv5 using packed
>> structures) with an arm-eabi cross compiler, and looked for differences in
>> the
>> disassembled code with and without this patch, but there were none.
>>
>> OK for trunk?
>
> I agree that the existing movmisaling path that you remove is simply bogus, so
> removing it looks fine to me. Can you give rationale to
>
> @@ -4773,6 +4738,8 @@ expand_assignment (tree to, tree from, b
> if (MEM_P (to_rtx)
> && GET_MODE (to_rtx) == BLKmode
> && GET_MODE (XEXP (to_rtx, 0)) != VOIDmode
> + && bitregion_start == 0
> + && bitregion_end == 0
> && bitsize > 0
> && (bitpos % bitsize) == 0
> && (bitsize % GET_MODE_ALIGNMENT (mode1)) == 0
>
> and especially to
>
> @@ -9905,7 +9861,7 @@ expand_expr_real_1 (tree exp, rtx target
> && modifier != EXPAND_STACK_PARM
> ? target : NULL_RTX),
> VOIDmode,
> - modifier == EXPAND_SUM ? EXPAND_NORMAL : modifier);
> + EXPAND_MEMORY);
>
> /* If the bitfield is volatile, we want to access it in the
> field's mode, not the computed mode.
>
> which AFAIK makes "memory" expansion of loads/stores from/to registers
> change (fail? go through stack memory?) - see handling of non-MEM return
> values from that expand_expr call.
In particular this seems to disable all movmisalign handling for MEM_REFs
wrapped in component references which looks wrong. I was playing with
typedef long long V
__attribute__ ((vector_size (2 * sizeof (long long)), may_alias));
struct S { long long a[11]; V v; }__attribute__((aligned(8),packed)) ;
struct S a, *b = &a;
V v, w;
int main()
{
v = b->v;
b->v = w;
return 0;
}
(use -fno-common) and I see that we use unaligned stores too often
(even with a properly aligned MEM).
The above at least shows movmisalign opportunities wrapped in component-refs.
> That is, do you see anything break with just removing the movmisalign path?
> I'd rather install that (with the new testcases that then pass) separately as
> this is a somewhat fragile area and being able to more selectively
> bisect/backport
> would be nice.
>
> Thanks,
> Richard.
>
>> Regards
>> Bernd.
