Project

General

Profile

Bug #2990

Deprecate Armv7

Added by Szilárd Páll about 1 year ago. Updated 9 months ago.

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

Description

On a NEON build with gcc 8 (on jetson tk1) I get:

gmx: /nethome/pszilard/gromacs-master/src/gromacs/simd/impl_arm_neon/impl_arm_neon_simd4_float.h:78: gmx::Simd4Float gmx::load4(const float*): Assertion `size_t(m) % 16 == 0' failed.
Aborted


Related issues

Precedes GROMACS - Bug #3274: Remove ARM NEON SIMDNew

Associated revisions

Revision fc017696 (diff)
Added by Mark Abraham 9 months ago

Stop testing ARMv7

Refs #2990

Change-Id: I837daf20df266a44ca53f7368a05a0cae36bc47d

Revision d5e465a3 (diff)
Added by Mark Abraham 9 months ago

Remove support for ARMv7

Includes partial revert of 0d4ea6033cba8cbfe6

Refs #2990

Change-Id: I7f154a15f382a53af89b9f8b5370bffbe47bfe66

Revision befea4fc (diff)
Added by Paul Bauer 9 months ago

Deprecate support for Armv7

Fixes #2990

Change-Id: Ia9dbfc9ea6cf23782e812ef2929e4851b264bc82

Revision 9f283b4d (diff)
Added by Mark Abraham 9 months ago

Remove GMX_CYCLECOUNTERS

With ARMv7 support removed, cycle counters are available on all
known platforms, so the CMake option to disable them can be removed.
This change will be easy to revert if the need arises in future.

Includes partial revert of 0d4ea6033cba8cbfe6, which was added
when supporting quirks of ARMv7.

Refs #2990

Change-Id: Iac107d977fcd8ea67d8306149b9fb6c17985afa4

History

#1 Updated by Paul Bauer about 1 year ago

I'm trying this again now and I no longer get the error but a whole bunch of warnings.
@Szilard is there still something to do here?

#2 Updated by Szilárd Páll about 1 year ago

Paul Bauer wrote:

I'm trying this again now and I no longer get the error but a whole bunch of warnings.
@Szilard is there still something to do here?

I've not tried and I can't prioritize this for beta1.

#3 Updated by Paul Bauer 12 months ago

  • Target version changed from 2020-beta1 to 2020-beta2

#4 Updated by Paul Bauer 11 months ago

  • Target version changed from 2020-beta2 to 2020-beta3

bump

#5 Updated by Paul Bauer 10 months ago

  • Target version changed from 2020-beta3 to 2020-rc1

bump!

#6 Updated by Paul Bauer 10 months ago

This is the warning I get now (and a few more)
As far as I understand from reading about it this should be caused by different sizes of GMX_SIMD_ALIGNMENT and GMX_SIMD_FINT32_WIDTH

/home/jenkins/test-2020/gromacs/src/gromacs/simd/impl_arm_neon/impl_arm_neon_util_float.h: In function ‘void gmx::gatherLoadBySimdIntTranspose(const float*, gmx::SimdFInt32, gmx::SimdFloat*, gmx::SimdFloat*, gmx::SimdFloat*, gmx::SimdFloat*)’:
/home/jenkins/test-2020/gromacs/src/gromacs/simd/impl_arm_neon/impl_arm_neon_util_float.h:274:75: warning: requested alignment 16 is larger than 8 [-Wattributes]
     alignas(GMX_SIMD_ALIGNMENT) std::int32_t ioffset[GMX_SIMD_FINT32_WIDTH];

#7 Updated by Berk Hess 10 months ago

Paul Bauer wrote:

This is the warning I get now (and a few more)
As far as I understand from reading about it this should be caused by different sizes of GMX_SIMD_ALIGNMENT and GMX_SIMD_FINT32_WIDTH
[...]

No, those are both 16.

Somehow the compiler seems to want to have an alignment of 8 instead of 16. I don't know why the compiler wants 8. 16 should give better performance.

#8 Updated by Paul Bauer 10 months ago

this should be the result of std::max_align_t being not correct for this case according to my google searches, but I don't get why it becomes an issue here

#9 Updated by Mark Abraham 10 months ago

ARMv7 doesn't actually have 128-bit registers, so our x86-based assumptions do not apply

#10 Updated by Mark Abraham 10 months ago

Paul Bauer wrote:

this should be the result of std::max_align_t being not correct for this case according to my google searches, but I don't get why it becomes an issue here

That constant is 8, because that's necessary for double to load on ARMv7. But it only documents the maximum alignment needed for loading any scalar type, which is a different thing from the alignment necessary for SIMD-style loads. The warning would be useful if we were allocating lots of excessively aligned things, but we don't. There's also a pile of other warnings with recent gcc on ARMv7 that aren't worth fixing. Since there's never been a relevant ARMv7 system, and won't be one, we should retire supporting and testing this arch before releasing 2020.

#11 Updated by Mark Abraham 9 months ago

These warnings could be suppressed with "-Wno-attributes" for ARMv7 if we want. But as there are numerous other warnings issued by gcc 8 on ARMv7 that look benign, and as ARMv7 is not an interesting platform for HPC, I suggest we either announce it as deprecated in GROMACS 2020. Or just drop it outright.

#12 Updated by Paul Bauer 9 months ago

  • Assignee set to Paul Bauer
  • Target version changed from 2020-rc1 to 2020

I'll add it to the deprecation list

#13 Updated by Paul Bauer 9 months ago

  • Subject changed from ARM neon SIMD4 error to Deprecate Armv7
  • Status changed from New to Fix uploaded

#14 Updated by Mark Abraham 9 months ago

  • Precedes Bug #3274: Remove ARM NEON SIMD added

#15 Updated by Paul Bauer 9 months ago

  • Status changed from Fix uploaded to Resolved

#16 Updated by Paul Bauer 9 months ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF