Project

General

Profile

Bug #1808

error: 'asm' operand has impossible constraints when compiling gromacs 5.1 on PPC64 and PPC64LE with VSX SIMD

Added by Dominik Mierzejewski about 5 years ago. Updated about 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
-
Target version:
Affected version - extra info:
Affected version:
Difficulty:
uncategorized
Close

Description

When building gromacs-5.1, I get the following compiler error:

[...]
[ 94%] Building CXX object src/gromacs/simd/tests/CMakeFiles/simd-test.dir/simd_integer.cpp.o
cd /builddir/build/BUILD/gromacs-5.1/openmpi_d/src/gromacs/simd/tests && /usr/bin/c++   -DGMX_DOUBLE -DGTEST_USE_OWN_TR1_TUPLE=1 -DHAVE_CONFIG_H -DTEST_DATA_PATH=\"src/gromacs/simd/tests\" -DTEST_TEMP_PATH=\"/builddir/build/BUILD/gromacs-5.1/openmpi_d/src/gromacs/simd/tests/Testing/Temporary\" -mvsx   -std=c++0x -O2 -g -pipe -Wall -Werror=format-security -Wp
,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64  -Wundef -Wextra -Wno-missing-field-initializers -Wpointer-arith -Wall -Wno-unused-function   -funroll-all-loops -fexcess-precision=fast  -Wno-array-bounds  -isystem /builddir/build/BUILD/gromacs-5.1/src/external/gmock-1.7.0/gtest/include -isystem /builddir/build/BUILD/gromacs-5.1/src/external/gmock-1.7.0/include -I/builddir/build/BUILD/gromacs-5.1/openmpi_d/src/external/tng_io/include -I/builddir/build/BUILD/gromacs-5.1/src/external/tng_io/include -I/builddir/build/BUILD/gromacs-5.1/openmpi_d/src -I/builddir/build/BUILD/gromacs-5.1/src/external/thread_mpi/include -I/builddir/build/BUILD/gromacs-5.1/src -I/usr/include/openmpi-ppc64    -Wno-unused-variable -o CMakeFiles/simd-test.dir/simd_integer.cpp.o -c /builddir/build/BUILD/gromacs-5.1/src/gromacs/simd/tests/simd_integer.cpp
In file included from /builddir/build/BUILD/gromacs-5.1/src/gromacs/simd/simd.h:138:0,
                 from /builddir/build/BUILD/gromacs-5.1/src/gromacs/simd/tests/simd_integer.cpp:37:
/builddir/build/BUILD/gromacs-5.1/src/gromacs/simd/impl_ibm_vsx/impl_ibm_vsx.h: In member function 'virtual void gmx::test::{anonymous}::SimdIntegerTest_gmxSimdCvtI2R_Test::TestBody()':
/builddir/build/BUILD/gromacs-5.1/src/gromacs/simd/impl_ibm_vsx/impl_ibm_vsx.h:452:80: error: 'asm' operand has impossible constraints
     __asm__ ("xvcvsxwdp %0,%1" : "=ww" (x) : "ww" ((__vector signed int) (ix)));
                                                                                ^
/builddir/build/BUILD/gromacs-5.1/src/gromacs/simd/impl_ibm_vsx/impl_ibm_vsx.h:452:80: error: 'asm' operand has impossible constraints
     __asm__ ("xvcvsxwdp %0,%1" : "=ww" (x) : "ww" ((__vector signed int) (ix)));
                                                                                ^
src/gromacs/simd/tests/CMakeFiles/simd-test.dir/build.make:209: recipe for target 'src/gromacs/simd/tests/CMakeFiles/simd-test.dir/simd_integer.cpp.o' failed
make[3]: *** [src/gromacs/simd/tests/CMakeFiles/simd-test.dir/simd_integer.cpp.o] Error 1


Full build logs attached.

Build environment: Fedora rawhide (F24)
gcc-5.1.1-4.fc23
fftw-3.3.4-6.fc23
boost-1.58.0-2.fc24
atlas-3.10.2-6.fc23
mpich-3.1.4-4.fc23
openmpi-1.8.7-1.fc24

ppc64-build.log (6.54 MB) ppc64-build.log Dominik Mierzejewski, 08/17/2015 02:35 PM
ppc64le-build.log (6.79 MB) ppc64le-build.log Dominik Mierzejewski, 08/17/2015 02:35 PM
gromacs-vsx.patch (744 Bytes) gromacs-vsx.patch Minimal patch to fix compilation Dominik Mierzejewski, 08/21/2015 12:53 AM
build.log (2.63 MB) build.log build log for serial and serial_d variants Dominik Mierzejewski, 08/21/2015 12:57 AM
gromacsasm.diff (2.31 KB) gromacsasm.diff Alan Modra, 08/21/2015 04:07 AM

Related issues

Related to GROMACS - Bug #1988: Double-precision SIMD test failure on powerpcClosed

Associated revisions

Revision 021cd76c (diff)
Added by Erik Lindahl almost 5 years ago

Extended SIMD, implementation for IBM Power7/8 VSX

Adds the extended/v2 SIMD layer. This version has been
tested on big-endian Power7 and little-endian Power8 with
gcc-4.9 in both single and double. The inline assembly
for gcc has been improved based on the proposals in
redmine.

Fixes #1808.

Change-Id: I0e52e847a5925a0329ae4be0cb04eb9fe6122fd9

History

#1 Updated by Dominik Mierzejewski about 5 years ago

Actually, the build itself completes fine. The above error is from "make check" part and the only with double precision. I'm running another build to confirm if it happens with openmpi only.

#2 Updated by Dominik Mierzejewski about 5 years ago

Confirmed: the failing combination for make check is MPI enabled (both openmpi and mpich) and double precision enabled.

#3 Updated by Dominik Mierzejewski about 5 years ago

Sorry, the MPI is irrelevant. make check compilation part fails in every case with double precision enabled.

#4 Updated by Erik Lindahl about 5 years ago

Hi Dominik,

This might be the external assembler command that does not understand the "=ww" register constraint (which means VSX register).

Could you try doing as --version to check?

For the record, we've tested Power8 extensively with Ubuntu and gcc-4.9. Not sure about Fedora24, but Redhat has historically been a bit slow to move to the latest version of the developer tools.

#5 Updated by Mark Abraham about 5 years ago

  • Description updated (diff)

#6 Updated by Dominik Mierzejewski about 5 years ago

Hi, Erik.

# as --version
GNU assembler version 2.25.1-1.fc24
Copyright (C) 2014 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or later.
This program has absolutely no warranty.
This assembler was configured for a target of `ppc64-redhat-linux'.

Fedora rawhide has gcc-5.1.1 and binutils-2.25.1 in the buildroot.

#7 Updated by Dominik Mierzejewski about 5 years ago

https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html suggests the code might be wrong.

When using any of the register constraints (wa, wd, wf, wg, wh, wi, wj, wk, wl, wm, wp, wq, ws, wt, wu, wv, ww, or wy) that take VSX registers, you must use %x<n> in the template so that the correct register is used. Otherwise the register number output in the assembly file will be incorrect if an Altivec register is an operand of a VSX instruction that expects VSX register numbering.

    asm ("xvadddp %x0,%x1,%x2" : "=wa" (v1) : "wa" (v2), "wa" (v3));

is correct, but:

    asm ("xvadddp %0,%1,%2" : "=wa" (v1) : "wa" (v2), "wa" (v3));

is not correct.

The code in src/gromacs/simd/impl_ibm_vsx is using %0 instead of %x0. However, even using the correct register designation still leaves gcc barfing at the asm construct at line 452.

#9 Updated by Alan Modra about 5 years ago

I don't make any claims to be a VSX expert but I think this is just bad asm. See comments added to the gcc bug.

#10 Updated by Dominik Mierzejewski about 5 years ago

With this minimal patch, based on gcc developers' suggestions in the bug report above, the compilation succeeds, however the internal tests (make check) fail.

#11 Updated by Alan Modra about 5 years ago

There are quite a few more places that should replace ww constraints, and the diff in the previous comment lacks the changes identified in comment #8. Here's a more comprehensive fix of vsx asm, but totally untested.

#12 Updated by Roland Schulz about 5 years ago

We prefer any code suggestions uploaded to gerrit.gromacs.org

#13 Updated by Dominik Mierzejewski about 5 years ago

@Alan, I tested your patch and it makes the testsuite fail the same way as with my minimal patch.

#14 Updated by Erik Lindahl almost 5 years ago

Hi Dominik,

If you hang on a few more weeks I should have a version ready where I've been able to completely avoid inline asm (not entirely trivial due to compiler variations and bugs...), and where the verlet kernels actually use VSX SIMD too (previously we primarily used it together with CUDA, so we didn't really care about CPU-only performance last year).

#15 Updated by Dominik Mierzejewski almost 5 years ago

Erik Lindahl wrote:

Hi Dominik,

If you hang on a few more weeks I should have a version ready where I've been able to completely avoid inline asm (not entirely trivial due to compiler variations and bugs...), and where the verlet kernels actually use VSX SIMD too (previously we primarily used it together with CUDA, so we didn't really care about CPU-only performance last year).

I'm already able to do that if I specify -DGMX_SIMD=None, so I'm not sure what you mean.

#16 Updated by Mark Abraham almost 5 years ago

Using -DGMX_SIMD=None trades away a significant factor of CPU performance, which matters even when using GPUs. So while using it allows some progress to be made, it's a long way from good.

#17 Updated by Erik Lindahl almost 5 years ago

As Mark writes, disabling SIMD is a bad solution from the performance p-o-v.

Normally we try hard to avoid inline assembly, since it interferes with the compiler's optimization engine - it's much better to just rely on intrinsics. However, Power8 & VSX is still new enough that the compilers are a bit buggy/incomplete, so there are a handful of instructions for which intrinsics are not yet available in gcc - for these we need to use inline assembly. This has however been improved now, and in the latest version I only need it for double.

If you want to help with testing, have a look at https://gerrit.gromacs.org/#/c/4551/ and let me know if it works for you. This version should have updated inline assembly, and it will also have nonbonded kernels that actually use the SIMD layer for Power8. It passes all regression tests in single and double with gcc-4.9.1 on both Ubuntu 15.04 (little-endian Power7) and AIX (big-endian Power8).

#18 Updated by Gerrit Code Review Bot almost 5 years ago

Gerrit received a related patchset '10' for Issue #1808.
Uploader: Erik Lindahl ()
Change-Id: I0e52e847a5925a0329ae4be0cb04eb9fe6122fd9
Gerrit URL: https://gerrit.gromacs.org/4551

#19 Updated by Erik Lindahl almost 5 years ago

  • Status changed from New to Resolved

#20 Updated by Erik Lindahl over 4 years ago

  • Status changed from Resolved to Closed

#21 Updated by Dominik Mierzejewski over 4 years ago

Erik Lindahl wrote:

Applied in changeset 021cd76cbed9148e385931fcb7014d680993d3d7.

This patch doesn't apply to 5.1.2. Are you going to backport this to 5.1.x branch?

#22 Updated by Mark Abraham about 4 years ago

  • Related to Bug #1988: Double-precision SIMD test failure on powerpc added

#23 Updated by Mark Abraham about 4 years ago

  • Target version set to 2016

I doubt we will spend the time to backport this fix to release-5-1 for this architecture, so I suggest using -DGMX_SIMD=None for repository builds.

Also available in: Atom PDF