[AArch64] Fix data race on RegisterBank initialization.

[huihuiz]

Summary

The initialization of RegisterBank needs to be done only once. The
logic of AlreadyInit has a data race, use llvm::call_once instead.

This issue was identified through thread sanitizer.

[huihuiz]

This issue was exposed when linking with ThinLTO+lld, using a compiler built with thread sanitizer enabled.

[efriedma]

Using std::call_once, instead of a separate flag+mutex, would be a little more readable and efficient.

[huihuiz]

Thanks Eli for the feedback!

[efriedma]

LGTM

[arsenm]

The other targets also do this

[huihuiz]

Thanks @arsenm for catching this!
Fixing in D73604 and D73605

llvm/lib/Target/AArch64/AArch64RegisterBankInfo.cpp

 
 // This file will be TableGen'ed at some point.
 #include "AArch64GenRegisterBankInfo.def"
 
 using namespace llvm;
 
 AArch64RegisterBankInfo::AArch64RegisterBankInfo(const TargetRegisterInfo &TRI)
     : AArch64GenRegisterBankInfo() {
-  static bool AlreadyInit = false;
+  static llvm::once_flag InitializeRegisterBankFlag;
+
+  static auto InitializeRegisterBankOnce = [this](const auto &TRI) {
     // We have only one set of register banks, whatever the subtarget
     // is. Therefore, the initialization of the RegBanks table should be
     // done only once. Indeed the table of all register banks
     // (AArch64::RegBanks) is unique in the compiler. At some point, it
     // will get tablegen'ed and the whole constructor becomes empty.
-  if (AlreadyInit)
-    return;
-  AlreadyInit = true;
 
     const RegisterBank &RBGPR = getRegBank(AArch64::GPRRegBankID);
     (void)RBGPR;
     assert(&AArch64::GPRRegBank == &RBGPR &&
            "The order in RegBanks is messed up");
 
     const RegisterBank &RBFPR = getRegBank(AArch64::FPRRegBankID);
     (void)RBFPR;
     assert(&AArch64::FPRRegBank == &RBFPR &&
            "The order in RegBanks is messed up");
 
     const RegisterBank &RBCCR = getRegBank(AArch64::CCRegBankID);
     (void)RBCCR;
-  assert(&AArch64::CCRegBank == &RBCCR && "The order in RegBanks is messed up");
+    assert(&AArch64::CCRegBank == &RBCCR &&
+           "The order in RegBanks is messed up");
 
     // The GPR register bank is fully defined by all the registers in
     // GR64all + its subclasses.
     assert(RBGPR.covers(*TRI.getRegClass(AArch64::GPR32RegClassID)) &&
            "Subclass not added?");
     assert(RBGPR.getSize() == 64 && "GPRs should hold up to 64-bit");
 
     // The FPR register bank is fully defined by all the registers in
     // GR64all + its subclasses.
     assert(RBFPR.covers(*TRI.getRegClass(AArch64::QQRegClassID)) &&
            "Subclass not added?");
     assert(RBFPR.covers(*TRI.getRegClass(AArch64::FPR64RegClassID)) &&
            "Subclass not added?");
     assert(RBFPR.getSize() == 512 &&
            "FPRs should hold up to 512-bit via QQQQ sequence");
 
     assert(RBCCR.covers(*TRI.getRegClass(AArch64::CCRRegClassID)) &&
            "Class not added?");
     assert(RBCCR.getSize() == 32 && "CCR should hold up to 32-bit");
 
     // Check that the TableGen'ed like file is in sync we our expectations.
     // First, the Idx.
     assert(checkPartialMappingIdx(PMI_FirstGPR, PMI_LastGPR,
                                   {PMI_GPR32, PMI_GPR64}) &&
            "PartialMappingIdx's are incorrectly ordered");
     assert(checkPartialMappingIdx(PMI_FirstFPR, PMI_LastFPR,
                                   {PMI_FPR16, PMI_FPR32, PMI_FPR64, PMI_FPR128,
                                    PMI_FPR256, PMI_FPR512}) &&
            "PartialMappingIdx's are incorrectly ordered");
 // Now, the content.
 // Check partial mapping.
 #define CHECK_PARTIALMAP(Idx, ValStartIdx, ValLength, RB)                      \
   do {                                                                         \
     assert(                                                                    \
         checkPartialMap(PartialMappingIdx::Idx, ValStartIdx, ValLength, RB) && \
         #Idx " is incorrectly initialized");                                   \
   } while (false)
 
     CHECK_PARTIALMAP(PMI_GPR32, 0, 32, RBGPR);
     CHECK_PARTIALMAP(PMI_GPR64, 0, 64, RBGPR);
     CHECK_PARTIALMAP(PMI_FPR16, 0, 16, RBFPR);
     CHECK_PARTIALMAP(PMI_FPR32, 0, 32, RBFPR);
     CHECK_PARTIALMAP(PMI_FPR64, 0, 64, RBFPR);
     CHECK_PARTIALMAP(PMI_FPR128, 0, 128, RBFPR);
     CHECK_PARTIALMAP(PMI_FPR256, 0, 256, RBFPR);
     CHECK_PARTIALMAP(PMI_FPR512, 0, 512, RBFPR);
 
 // Check value mapping.
 #define CHECK_VALUEMAP_IMPL(RBName, Size, Offset)                              \
   do {                                                                         \
     assert(checkValueMapImpl(PartialMappingIdx::PMI_##RBName##Size,            \
                              PartialMappingIdx::PMI_First##RBName, Size,       \
                              Offset) &&                                        \
            #RBName #Size " " #Offset " is incorrectly initialized");           \
   } while (false)
 
 #define CHECK_VALUEMAP(RBName, Size) CHECK_VALUEMAP_IMPL(RBName, Size, 0)
 
     CHECK_VALUEMAP(GPR, 32);
     CHECK_VALUEMAP(GPR, 64);
     CHECK_VALUEMAP(FPR, 16);
     CHECK_VALUEMAP(FPR, 32);
     CHECK_VALUEMAP(FPR, 64);
     CHECK_VALUEMAP(FPR, 128);
     CHECK_VALUEMAP(FPR, 256);
     CHECK_VALUEMAP(FPR, 512);
 
 // Check the value mapping for 3-operands instructions where all the operands
 // map to the same value mapping.
 #define CHECK_VALUEMAP_3OPS(RBName, Size)                                      \
   do {                                                                         \
     CHECK_VALUEMAP_IMPL(RBName, Size, 0);                                      \
     CHECK_VALUEMAP_IMPL(RBName, Size, 1);                                      \
     CHECK_VALUEMAP_IMPL(RBName, Size, 2);                                      \
   } while (false)
 
     CHECK_VALUEMAP_3OPS(GPR, 32);
     CHECK_VALUEMAP_3OPS(GPR, 64);
     CHECK_VALUEMAP_3OPS(FPR, 32);
     CHECK_VALUEMAP_3OPS(FPR, 64);
     CHECK_VALUEMAP_3OPS(FPR, 128);
     CHECK_VALUEMAP_3OPS(FPR, 256);
     CHECK_VALUEMAP_3OPS(FPR, 512);
 
 #define CHECK_VALUEMAP_CROSSREGCPY(RBNameDst, RBNameSrc, Size)                 \
   do {                                                                         \
     unsigned PartialMapDstIdx = PMI_##RBNameDst##Size - PMI_Min;               \
     unsigned PartialMapSrcIdx = PMI_##RBNameSrc##Size - PMI_Min;               \
     (void)PartialMapDstIdx;                                                    \
     (void)PartialMapSrcIdx;                                                    \
     const ValueMapping *Map = getCopyMapping(                                  \
         AArch64::RBNameDst##RegBankID, AArch64::RBNameSrc##RegBankID, Size);  \
     (void)Map;                                                                 \
     assert(Map[0].BreakDown ==                                                 \
                &AArch64GenRegisterBankInfo::PartMappings[PartialMapDstIdx] &&  \
            Map[0].NumBreakDowns == 1 && #RBNameDst #Size                       \
            " Dst is incorrectly initialized");                                 \
     assert(Map[1].BreakDown ==                                                 \
                &AArch64GenRegisterBankInfo::PartMappings[PartialMapSrcIdx] &&  \
            Map[1].NumBreakDowns == 1 && #RBNameSrc #Size                       \
            " Src is incorrectly initialized");                                 \
                                                                                \
   } while (false)
 
     CHECK_VALUEMAP_CROSSREGCPY(GPR, GPR, 32);
     CHECK_VALUEMAP_CROSSREGCPY(GPR, FPR, 32);
     CHECK_VALUEMAP_CROSSREGCPY(GPR, GPR, 64);
     CHECK_VALUEMAP_CROSSREGCPY(GPR, FPR, 64);
     CHECK_VALUEMAP_CROSSREGCPY(FPR, FPR, 32);
     CHECK_VALUEMAP_CROSSREGCPY(FPR, GPR, 32);
     CHECK_VALUEMAP_CROSSREGCPY(FPR, FPR, 64);
     CHECK_VALUEMAP_CROSSREGCPY(FPR, GPR, 64);
 
 #define CHECK_VALUEMAP_FPEXT(DstSize, SrcSize)                                 \
   do {                                                                         \
     unsigned PartialMapDstIdx = PMI_FPR##DstSize - PMI_Min;                    \
     unsigned PartialMapSrcIdx = PMI_FPR##SrcSize - PMI_Min;                    \
     (void)PartialMapDstIdx;                                                    \
     (void)PartialMapSrcIdx;                                                    \
     const ValueMapping *Map = getFPExtMapping(DstSize, SrcSize);               \
     (void)Map;                                                                 \
     assert(Map[0].BreakDown ==                                                 \
                &AArch64GenRegisterBankInfo::PartMappings[PartialMapDstIdx] &&  \
            Map[0].NumBreakDowns == 1 && "FPR" #DstSize                         \
                                         " Dst is incorrectly initialized");    \
     assert(Map[1].BreakDown ==                                                 \
                &AArch64GenRegisterBankInfo::PartMappings[PartialMapSrcIdx] &&  \
            Map[1].NumBreakDowns == 1 && "FPR" #SrcSize                         \
                                         " Src is incorrectly initialized");    \
                                                                                \
   } while (false)
 
     CHECK_VALUEMAP_FPEXT(32, 16);
     CHECK_VALUEMAP_FPEXT(64, 16);
     CHECK_VALUEMAP_FPEXT(64, 32);
     CHECK_VALUEMAP_FPEXT(128, 64);
 
     assert(verify(TRI) && "Invalid register bank information");
+  };
+
+  llvm::call_once(InitializeRegisterBankFlag, InitializeRegisterBankOnce, TRI);
 }
 
 unsigned AArch64RegisterBankInfo::copyCost(const RegisterBank &A,
                                            const RegisterBank &B,
                                            unsigned Size) const {
   // What do we do with different size?
   // copy are same size.
   // Will introduce other hooks for different size: