This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Comprehensive tests for atomic operations
ClosedPublic

Authored by tmatheson on Jan 5 2023, 7:54 AM.

Details

Summary

There are a lot of variants of atomic operations, and AArch64 has several
distinct options to codegen them, and does different things depending on
available features, architecture version and optimisation level. The current
testing for atomic operations has been added gradually over time and does not
give full coverate. Given how complex the codegen for atomic operations is, it
is valuable to cover the entire parameter space, i.e. test them all. The
resulting set of tests serve also as a reference for how each is codegened.

In order to keep the test files readable and avoid constant updating for
unrelated codegen changes, the test outputs are filtered to only include the
relevant instructions. This shows for each operation and feature which codegen
approach is taken (e.g. ll/sc loop, atomic instruction, library call).

The following parameter space is tested:

  • feature: +lse, +rcpc, etc
  • optimisation level: O0, O1 (covers GISel and SelectionDAG)
  • atomic instruction: load, store, cmpxchg, atomirmw*
  • size: i8, i16, i32, i64, i128
  • aligned/unaligned accesses
  • endianness: big, little
  • atomic ordering: release, acquire, etc
  • load atomic only: const/non-const
  • cmpxchg only: weak/strong
  • atomicrmw: update operation (add, sub, etc)

Notably not covered:

  • volatility: there should be no difference between volatile/non-volatile
  • atomicrmw fadd/fsub

The files are split by triple, LLVM instruction, and feature. This makes it
easy to diff between features and endianness for a given instruction.

The file that generates the tests is included.

There are 70 test files with an average of 2800 lines each.

Diff Detail

Event Timeline

tmatheson created this revision.Jan 5 2023, 7:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 5 2023, 7:54 AM
tmatheson requested review of this revision.Jan 5 2023, 7:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 5 2023, 7:54 AM
lenary accepted this revision.Jan 6 2023, 3:41 AM

I am very pro this change - we have had these tests downstream for a few months, and they've helped us catch changes in codegen we might not otherwise have noticed.

As we're about to work on codegen changes for lse128 and rcpc3 (part of the 2022 extensions), I think we have enough different configurations to warrant these comprehensive tests. There are, of course, more configurations we could add in the future, and the generate-tests.py script should allow us to do so easily.

I am happy that these are split up enough to not cause very slow tests.

I commented on one Nit, below, but I think you can fix it when you commit these.

llvm/test/CodeGen/AArch64/Atomics/generate-tests.py
245

Nit: This writes the header into the fence test files multiple times, leaving The base test file was generated by ./llvm/test/CodeGen/AArch64/Atomics/generate-tests.py printed multiple times.

This revision is now accepted and ready to land.Jan 6 2023, 3:41 AM
This revision was landed with ongoing or failed builds.Jan 10 2023, 6:02 AM
This revision was automatically updated to reflect the committed changes.

Now that we have the comprehensive tests, does it make sense to remove any of the existing tests for atomics?

It does, I need to check that the old tests are definitely replicated here before removing them.