This is an archive of the discontinued LLVM Phabricator instance.

openmp: Align kmp_int64 and kmp_uint64 to 64 bits.
Needs ReviewPublic

Authored by pcc on Apr 18 2019, 3:53 PM.

Details

Reviewers
jlpeyton
Summary

On 32-bit x86, both of these types have an alignment of 32 bits, which means
that atomic operations involving these types require a runtime call. We can
avoid the overhead of the runtime call, as well as a dependency on libatomic,
by aligning both types to 64 bits.

As far as I can tell these types are not part of the library's public
interface, so this change should be fine from an ABI perspective.

Event Timeline

pcc created this revision.Apr 18 2019, 3:53 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 18 2019, 3:53 PM

Not sure this is a good change, e.g. gcc produces tons of warnings:

warning: ignoring attributes on template argument 'kmp_uint64 {aka long long unsigned int}' [-Wignored-attributes]
  std::atomic<kmp_uint64> next_ticket;
                        ^

and apparently ignores the attribute. Strictly, C++ does not allow to specify alignment in typedef, AFAIK. Though many compilers may support this.

Maybe it would be better to align objects, not types. There should not be many 64-bit atomic objects in the code those need explicit alignment, I think. And there are places where (poorly aligned) pair of 4-byte integers is changes atomically as a single 8-bype object. We may need to reorder structure fields then, as aligned type won't help here.

Just my 2c.