On Fri, Sep 6, 2013 at 10:35 AM, Bernd Edlinger <bernd.edlin...@hotmail.de> wrote: > Richard, > >> But the movmisalign path skips all this code and with the >> current code thinks the actual access is in the mode of the >> whole structure. (and also misses the address adjustment >> as shown in the other testcase for the bug) >> >> The movmisalign handling in this path is just broken. And >> it's _not_ just for optimization! If you have >> >> struct __attribute__((packed)) { >> double x; >> v2df y; >> } *p; >> >> and do >> >> p = malloc (48); // gets you 8-byte aligned memory >> p->y = (v2df) { 1., 2. }; >> >> then you cannot skip the movmisaling handling because >> we'd generate an aligned move (based on the mode) then. >> > > Ok, test examples are really helpful here. > > This time the structure is BLKmode, unaligned, > movmisalign = false anyway. > > I tried to make a test case out of your example, > and as I expected, the code is also correct: > > foo: > .cfi_startproc > movdqa .LC1(%rip), %xmm0 > movq $-1, (%rdi) > movl $0x4048f5c3, 8(%rdi) > movdqu %xmm0, 12(%rdi) > ret > > movdqu. > > The test executes without trap. > And I did everything to make the object unaligned.
Yeah, well - for the expand path with BLKmode it's working fine. We're doing all the dances because of correctness for non-BLKmode expand paths. > I am sure we could completely remove the > movmisalign path, and nothing would happen. > probably. except maybe for a performance regression. In this place probably yes. But we can also fix it to use the proper mode and properly do the offset adjustment. But just adding a bounds check looks broken to me. Note that there may have been a correctness reason for this code in the light of the IPA-SRA code. Maybe Martin remembers. If removing the movmisalign handling inside the handled-component case in expand_assignment works out (make sure to also test a strict-alignment target) then that is probably fine. Richard. > > Bernd.