On Tue, Sep 17, 2013 at 12:00 PM, Richard Biener <richard.guent...@gmail.com> wrote: > On Sun, Sep 15, 2013 at 6:55 PM, Bernd Edlinger > <bernd.edlin...@hotmail.de> 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.