Comments

On Fri, Oct 12, 2012 at 7:57 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Hello,>> I was running a couple of tests on various platforms in preparation> of getting the find_reload_subreg_address patch needed by aarch64 upstream:> http://gcc.gnu.org/ml/gcc-patches/2012-07/msg01421.html>> This unfortunately uncovered a regression in vect-98-big-array.c on i386.> It seems to me that this is due to a pre-existing problem in the back-end.
[...]
> But in principle the problem is latent even without the patch. The back-end> should not permit reload to make changes to the pattern that fundamentally> change the semantics of the resulting instruction ...>> I was wondering if the i386 port maintainers could have a look at this> pattern. Shouldn't we really have two patterns, one to *load* an unaligned> value and one to *store* and unaligned value, and not permit that memory> access to get reloaded?
Please find attached a fairly mechanical patch that splits
move_unaligned pattern into load_unaligned and store_unaligned
patterns. We've had some problems with this pattern, and finally we
found the reason to make unaligned moves more robust.
I will wait for the confirmation that attached patch avoids the
failure you are seeing with your reload patch.
> [I guess a more fundamental change might also be to not have an unspec> in the first place, but only plain moves, and check MEM_ALIGN in the> move insn emitter to see which variant of the instruction is required ...]
We already do this for AVX moves, but SSE aligned moves should ICE on
unaligned access. This is handy to find errors in user programs (or
... well ... in the compiler itself).
Uros.

On Mon, Oct 15, 2012 at 6:39 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
>> On Fri, Oct 12, 2012 at 7:57 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:>> > I was wondering if the i386 port maintainers could have a look at this>> > pattern. Shouldn't we really have two patterns, one to *load* an unaligned>> > value and one to *store* and unaligned value, and not permit that memory>> > access to get reloaded?>>>> Please find attached a fairly mechanical patch that splits>> move_unaligned pattern into load_unaligned and store_unaligned>> patterns. We've had some problems with this pattern, and finally we>> found the reason to make unaligned moves more robust.>>>> I will wait for the confirmation that attached patch avoids the>> failure you are seeing with your reload patch.>> Yes, this patch does in fact fix the failure I was seeing with the> reload patch. (A full regression test shows a couple of extra fails:> FAIL: gcc.target/i386/avx256-unaligned-load-1.c scan-assembler sse_movups/1> FAIL: gcc.target/i386/avx256-unaligned-load-3.c scan-assembler sse2_movupd/1> FAIL: gcc.target/i386/avx256-unaligned-load-4.c scan-assembler avx_movups256/1> FAIL: gcc.target/i386/avx256-unaligned-store-4.c scan-assembler avx_movups256/2> But I guess these tests simply need to be updated for the new pattern names.)
Yes, the complete patch I am about to send to gcc-patches@ ML will
include all testsuite adjustments.
Uros.