gcc.target/arm/neon test regressions

[Andrew Stubbs]

Linaro GCC has some test regressions vs. Ubuntu FSF GCC:

+FAIL: gcc.target/arm/neon/vadds64.c scan-assembler vadd.i64[\\t]+[dD][0-9]+, [dD][0-9]+, [dD][0-9]+!?([ \\t]+@[a-zA-Z0-9 ]+)?\\n
+FAIL: gcc.target/arm/neon/vaddu64.c scan-assembler vadd.i64[\\t]+[dD][0-9]+, [dD][0-9]+, [dD][0-9]+!?([ \\t]+@[a-zA-Z0-9 ]+)?\\n
+FAIL: gcc.target/arm/neon/vands64.c scan-assembler vand[\\t]+[dD][0-9]+, [dD][0-9]+, [dD][0-9]+!?([ \\t]+@[a-zA-Z0-9 ]+)?\\n
+FAIL: gcc.target/arm/neon/vandu64.c scan-assembler vand[\\t]+[dD][0-9]+, [dD][0-9]+, [dD][0-9]+!?([ \\t]+@[a-zA-Z0-9 ]+)?\\n
+FAIL: gcc.target/arm/neon/veors64.c scan-assembler veor[\\t]+[dD][0-9]+, [dD][0-9]+, [dD][0-9]+!?([ \\t]+@[a-zA-Z0-9 ]+)?\\n
+FAIL: gcc.target/arm/neon/veoru64.c scan-assembler veor[\\t]+[dD][0-9]+, [dD][0-9]+, [dD][0-9]+!?([ \\t]+@[a-zA-Z0-9 ]+)?\\n
+FAIL: gcc.target/arm/neon/vorrs64.c scan-assembler vorr[\\t]+[dD][0-9]+, [dD][0-9]+, [dD][0-9]+!?([ \\t]+@[a-zA-Z0-9 ]+)?\\n
+FAIL: gcc.target/arm/neon/vorru64.c scan-assembler vorr[\\t]+[dD][0-9]+, [dD][0-9]+, [dD][0-9]+!?([ \\t]+@[a-zA-Z0-9 ]+)?\\n
+FAIL: gcc.target/arm/neon/vsubs64.c scan-assembler vsub.i64[\\t]+[dD][0-9]+, [dD][0-9]+, [dD][0-9]+!?([ \\t]+@[a-zA-Z0-9 ]+)?\\n
+FAIL: gcc.target/arm/neon/vsubu64.c scan-assembler vsub.i64[\\t]+[dD][0-9]+, [dD][0-9]+, [dD][0-9]+!?([ \\t]+@[a-zA-Z0-9 ]+)?\\n

These are for the ARMel toolchain only, of course.

There are patches upstream to fix these:
 http://gcc.gnu.org/ml/gcc-patches/2010-06/msg02101.html
 http://gcc.gnu.org/ml/gcc-patches/2010-06/msg02102.html

[Andrew Stubbs (ams-codesourcery)]

Julian, we agreed you would backport these patches.

[Loïc Minier (lool)]

Joseph Myers from CodeSourcery commented:
CS #8399; Thumb-2 specific but Ubuntu defaults to Thumb-2; being fixed upstream by Sandra. Sandra sent the fix:
http://gcc.gnu.org/ml/gcc-patches/2010-06/msg02101.html
http://gcc.gnu.org/ml/gcc-patches/2010-06/msg02102.html

[Michael Hope (michaelh1)]

Confirmed on 4.4.3-3ubuntu4~ppa1 (doko/2010-03-16)

[Michael Hope (michaelh1)]

Sorry, make that gcc-4.4_4.4.4-6ubuntu5~ppa2

[Ulrich Weigand (uweigand)]

*Not* confirmed in a direct gcc-linaro source build; all these tests pass for me.

I'm now trying to reproduce the bug with the gcc-4.4_4.4.4-6ubuntu5~ppa2 package.

[Loïc Minier (lool)]

To clarify: the regression is between the Ubuntu GCC packages built from FSF GCC without the Linaro diff. Once one adds the Linaro changes to the Ubuntu package, the regressions appear.

[Ulrich Weigand (uweigand)]

OK, I can reproduce the problem now with a gcc-linaro source build when using -march=armv7-a. This is the default for the compiler built by the Ubuntu package.

With -march=armv7-a added, the test cases pass in FSF GCC 4.4, but fail in Linaro GCC 4.4. This is due to a portion of the Linaro patch that enables more optimization of the Neon builtins.

The relevant patches have already been accepted upstream. For further discussion, see:
http://gcc.gnu.org/ml/gcc-patches/2010-05/msg02262.html
http://gcc.gnu.org/ml/gcc-patches/2010-06/msg02101.html

To summarize briefly, the original GCC 4.4 compiler would implement (for example) the vadd_s64 builtin by ensuring the vadd.i64 instruction is generated. GCC mainline, and the Linaro GCC 4.4, however, recognize that the vadd_s64 semantically simply means to perform a 64-bit integer addition, and then go ahead and generate whatever code sequence seems optimal to implement this semantics under any particular circumstances.

This may end up being in fact a vadd.i64 instruction, but it may also be some other sequence. This is what happens in the test case above; if the armv7-a instruction set is available, the compiler considers it more efficient to hold the operands in regular registers and then generate a piecewise addition using adds / adc, instead of moving the operands to Neon registers and then using vadd.i64.

This behaviour is compatible with the description of the intrinsics, so it is not a compiler bug. And in fact, when the patches were merged upstream, the test cases refered to in this bug report have been disabled, and replaced by tests that verify that the semantically correct operation is performed (instead of verifying that a particular instruction is generated).

The only remaining problem, it seems, is that these testcase changes, while committed upstream, have not been part of the Linaro 4.4 patch set. I'd suggest to leave Linaro GCC 4.4 as-is for the release, and backport those testcase changes for Linaro GCC 4.5.

[Loïc Minier (lool)]

Ok, lowering severity as code generation is actually correct, and only the testsuite needs updating.

Ulrich, would this be hard to backport to Linaro GCC 4.4?

[Ulrich Weigand (uweigand)]

The attached patch is a backport of the missing neon.ml changes and associated test suite changes from the upstream patches posted here:
http://gcc.gnu.org/ml/gcc-patches/2010-06/msg02100.html
http://gcc.gnu.org/ml/gcc-patches/2010-06/msg02101.html

This fixes the test suite regressions described in this bug.
All new test cases introduced by this patch pass.

[Michael Hope (michaelh1)]

fixed-in: 93536