This is an archive of the discontinued LLVM Phabricator instance.

tsan: align ThreadState to cache line
ClosedPublic

Authored by dvyukov on Sep 26 2021, 11:07 PM.

Details

Summary

There are 2 reasons to do this:

  1. We place hot data in the first cache line of ThreadState,

this assumed that it's cache-line-aligned but we never actually
enforced it (or it was lost at some point).

  1. The new vector clock uses vector instructions and requires

data alignment. Later the new vector clock will be embedded in
ThreadState, then ensuring vector clock alignment will be
impossible w/o ThreadState alignment.

Depends on D110519.

Diff Detail

Event Timeline

dvyukov requested review of this revision.Sep 26 2021, 11:07 PM
dvyukov created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptSep 26 2021, 11:07 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
melver accepted this revision.Sep 27 2021, 3:24 AM
melver added inline comments.
compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
148

I assume this could only fail if the allocator messes things up?

This revision is now accepted and ready to land.Sep 27 2021, 3:24 AM
dvyukov added inline comments.Sep 27 2021, 3:52 AM
compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
148

Yes, if something messes things up. Maybe linker or maybe one of our custom allocations (we frequently allocate things in strange ways).
I had some failures related to ThreadState alignment in tests, but I can't reproduce them and not sure if there was something real, or just an inconsistent build (cmake seems to mishandle some of test dependencies sometimes).

This revision was landed with ongoing or failed builds.Sep 27 2021, 3:54 AM
This revision was automatically updated to reflect the committed changes.

Hi Nemanja,

I hope this will fix the breakages:
https://reviews.llvm.org/D110629
If not, I think we need to remove the CHECK.
Sorry for the breakage.