openblas fails to compile on ppc64le-linux using gcc-7

[amodra]

From https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79343
kernel/power/sasum_microk_power8.c gives error: PIC register clobbered by ‘%2’ in ‘asm’

The blas source looks to be adding r2 (and r0) to the clobbers completely unnecessarily. I suspect what the author of that code was trying to do was say that i and n are changed by the asm, which is done by making them output args as well as input. The memory clobber could be removed too, if *x1 was made an output. Oh, and the asm misses clobbers for all the vector regs used..

[martin-frbg]

Similar reservations were noted in the mailing list discussion following (and linked from) #795. @wernsaar ?

[brada4]

Does it compile with current GCC release 6.3? GCC7 is development branch and expected to carry regressions.

[amodra]

So the first comment was wrong about *x1. I meant *svec, the output. Patch posted here:
https://groups.google.com/forum/#!topic/openblas-dev/YI1of4HODmw
Not tested apart from simply compiling. I'm going to leave testing to someone else.

Yes, the current source might compile with gcc-6 since gcc-6 hasn't switched over to lra by default for ppc, but the asm is horribly broken.

[martin-frbg]

"broken" in the sense that the current code gives wrong results, or just that it is written with (I guess) a strong x86 accent ?

[amodra]

On Fri, Feb 03, 2017 at 01:50:40AM -0800, Martin Kroeker wrote: "broken" in the sense that the current code gives wrong results, or just that it is written with (I guess) a strong x86 accent ?

The current code trashes callee saved regs fr14 to fr23. A function that accidentally modifies the variables of its callers (and not just the immediate caller) is horribly broken.

…

-- Alan Modra Australia Development Lab, IBM

[edelsohn]

@brada4 @martin-frbg The OpenBLAS VSX code is wrong and it is not a GCC 7 regression.

[martin-frbg]

Thank you both for the clarification. Given this situation I am inclined to merge the patch as-is although I am mostly illiterate when it comes to assembly and I have no ability to test it.

[martin-frbg]

At least 24 other files appear to be equally affected, and I do not feel confident to apply these corrections even to closely similar files like dasum_microk_power8.c, sorry.

[besser82]

Commenting here for being in the loop… See: https://bugzilla.redhat.com/show_bug.cgi?id=1417385

[amodra]

On Fri, Feb 03, 2017 at 02:23:35PM -0800, Martin Kroeker wrote: At least 24 other files appear to be equally affected

Indeed. What a mess! I'll take a look at fixing them all.

…

-- Alan Modra Australia Development Lab, IBM

[amodra]

For the record, it's more than just the gcc asm() statements. The assembly files are broken too, I think, using callee-saved altivec regs without saving them.
Oh, and the patch I posted wasn't correct either. Oops. The constraints were not quite right, and I should have put vs32 etc. in the asm clobbers, not plain 32 etc. In sasum_microk_power8.c fr14..fr23 were not being trashed but some altivec regs, v20, v21, v22 and v23.
In a lot of cases by using the right vsx registers we probably don't need to touch callee-saves. There are 34 vsx regs that don't need to be saved, vs0-vs13 (corresponding to fr0-fr13),and vs32-vs51 (corresponding to v0-v19).

[edelsohn]

Thanks for grabbing this, Alan. Getting inline asm correct is tricky.

[grisuthedragon]

@brada4 I tried it on pp64le with GCC 6.3 and it works except of the #1071 bug.

[martin-frbg]

@grisuthedragon as I understood amodra, the code delivers the expected result (else I am sure wernsaar would not have committed it) but may trash unrelated bits in the process. (The difference between GCC6 and GCC7 in this context appears to be simply that GCC6 was not yet able to diagnose such errors in inline ppc assembly)
(Is there some whitepaper on power8 assembly that explains how to code for this processor so "we" can avoid making the same mistakes in the future ?)