Skip to content

[llvm][OpenMP][NFC] Cleanup AtomicInfo #119199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 23, 2024

Conversation

NimishMishra
Copy link
Contributor

@NimishMishra NimishMishra commented Dec 9, 2024

This PR refactors functionality from llvm/include/llvm/Frontend/Atomic/Atomic.h into llvm/lib/llvm/Frontend/Atomic/Atomic.cpp.

@NimishMishra NimishMishra changed the title [llvm][NFC] Cleanup AtomicInfo [llvm][OpenMP][NFC] Cleanup AtomicInfo Dec 9, 2024
@llvmbot llvmbot added flang:openmp clang:openmp OpenMP related changes to Clang labels Dec 9, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 9, 2024

@llvm/pr-subscribers-flang-openmp

Author: None (NimishMishra)

Changes

This PR refactors functionality into Atomic.cpp.


Patch is 20.59 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/119199.diff

5 Files Affected:

  • (modified) llvm/include/llvm/Frontend/Atomic/Atomic.h (+36-157)
  • (modified) llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h (+7-16)
  • (modified) llvm/lib/Frontend/Atomic/Atomic.cpp (+142-2)
  • (modified) llvm/lib/Frontend/OpenMP/CMakeLists.txt (+1)
  • (modified) llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp (+2-2)
diff --git a/llvm/include/llvm/Frontend/Atomic/Atomic.h b/llvm/include/llvm/Frontend/Atomic/Atomic.h
index 3942d06144ce17..ce774b0df60185 100644
--- a/llvm/include/llvm/Frontend/Atomic/Atomic.h
+++ b/llvm/include/llvm/Frontend/Atomic/Atomic.h
@@ -12,6 +12,7 @@
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/IR/DataLayout.h"
+#include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/Module.h"
@@ -19,37 +20,37 @@
 #include "llvm/IR/RuntimeLibcalls.h"
 
 namespace llvm {
+class AtomicInfo;
 
-template <typename IRBuilderTy> struct AtomicInfo {
-
-  IRBuilderTy *Builder;
+class AtomicInfo {
+protected:
+  IRBuilderBase *Builder;
   Type *Ty;
   uint64_t AtomicSizeInBits;
   uint64_t ValueSizeInBits;
-  llvm::Align AtomicAlign;
-  llvm::Align ValueAlign;
+  Align AtomicAlign;
+  Align ValueAlign;
   bool UseLibcall;
 
 public:
-  AtomicInfo(IRBuilderTy *Builder, Type *Ty, uint64_t AtomicSizeInBits,
-             uint64_t ValueSizeInBits, llvm::Align AtomicAlign,
-             llvm::Align ValueAlign, bool UseLibcall)
+  AtomicInfo(IRBuilderBase *Builder, Type *Ty, uint64_t AtomicSizeInBits,
+             uint64_t ValueSizeInBits, Align AtomicAlign, Align ValueAlign,
+             bool UseLibcall)
       : Builder(Builder), Ty(Ty), AtomicSizeInBits(AtomicSizeInBits),
         ValueSizeInBits(ValueSizeInBits), AtomicAlign(AtomicAlign),
         ValueAlign(ValueAlign), UseLibcall(UseLibcall) {}
 
   virtual ~AtomicInfo() = default;
 
-  llvm::Align getAtomicAlignment() const { return AtomicAlign; }
+  Align getAtomicAlignment() const { return AtomicAlign; }
   uint64_t getAtomicSizeInBits() const { return AtomicSizeInBits; }
   uint64_t getValueSizeInBits() const { return ValueSizeInBits; }
   bool shouldUseLibcall() const { return UseLibcall; }
-  llvm::Type *getAtomicTy() const { return Ty; }
+  Type *getAtomicTy() const { return Ty; }
 
-  virtual llvm::Value *getAtomicPointer() const = 0;
+  virtual Value *getAtomicPointer() const = 0;
   virtual void decorateWithTBAA(Instruction *I) = 0;
-  virtual llvm::AllocaInst *CreateAlloca(llvm::Type *Ty,
-                                         const llvm::Twine &Name) const = 0;
+  virtual AllocaInst *CreateAlloca(Type *Ty, const Twine &Name) const = 0;
 
   /*
    * Is the atomic size larger than the underlying value type?
@@ -62,90 +63,28 @@ template <typename IRBuilderTy> struct AtomicInfo {
 
   LLVMContext &getLLVMContext() const { return Builder->getContext(); }
 
-  static bool shouldCastToInt(llvm::Type *ValTy, bool CmpXchg) {
-    if (ValTy->isFloatingPointTy())
-      return ValTy->isX86_FP80Ty() || CmpXchg;
-    return !ValTy->isIntegerTy() && !ValTy->isPointerTy();
-  }
+  bool shouldCastToInt(Type *ValTy, bool CmpXchg);
 
-  llvm::Value *EmitAtomicLoadOp(llvm::AtomicOrdering AO, bool IsVolatile,
-                                bool CmpXchg = false) {
-    Value *Ptr = getAtomicPointer();
-    Type *AtomicTy = Ty;
-    if (shouldCastToInt(Ty, CmpXchg))
-      AtomicTy = llvm::IntegerType::get(getLLVMContext(), AtomicSizeInBits);
-    LoadInst *Load =
-        Builder->CreateAlignedLoad(AtomicTy, Ptr, AtomicAlign, "atomic-load");
-    Load->setAtomic(AO);
-    if (IsVolatile)
-      Load->setVolatile(true);
-    decorateWithTBAA(Load);
-    return Load;
-  }
+  Value *EmitAtomicLoadOp(AtomicOrdering AO, bool IsVolatile,
+                          bool CmpXchg = false);
 
-  static CallInst *EmitAtomicLibcall(IRBuilderTy *Builder, StringRef fnName,
-                                     Type *ResultType, ArrayRef<Value *> Args) {
-    LLVMContext &ctx = Builder->getContext();
-    SmallVector<Type *, 6> ArgTys;
-    for (Value *Arg : Args)
-      ArgTys.push_back(Arg->getType());
-    FunctionType *FnType = FunctionType::get(ResultType, ArgTys, false);
-    Module *M = Builder->GetInsertBlock()->getModule();
-
-    // TODO: Use llvm::TargetLowering for Libcall ABI
-    llvm::AttrBuilder fnAttrBuilder(ctx);
-    fnAttrBuilder.addAttribute(llvm::Attribute::NoUnwind);
-    fnAttrBuilder.addAttribute(llvm::Attribute::WillReturn);
-    llvm::AttributeList fnAttrs = llvm::AttributeList::get(
-        ctx, llvm::AttributeList::FunctionIndex, fnAttrBuilder);
-    FunctionCallee LibcallFn = M->getOrInsertFunction(fnName, FnType, fnAttrs);
-    CallInst *Call = Builder->CreateCall(LibcallFn, Args);
-    return Call;
-  }
+  CallInst *EmitAtomicLibcall(IRBuilderBase *Builder, StringRef fnName,
+                              Type *ResultType, ArrayRef<Value *> Args);
 
-  llvm::Value *getAtomicSizeValue() const {
+  Value *getAtomicSizeValue() const {
     LLVMContext &ctx = getLLVMContext();
-
     // TODO: Get from llvm::TargetMachine / clang::TargetInfo
-    // 	if clang shares this codegen in future
+    // if clang shares this codegen in future
     constexpr uint16_t SizeTBits = 64;
     constexpr uint16_t BitsPerByte = 8;
-    return llvm::ConstantInt::get(llvm::IntegerType::get(ctx, SizeTBits),
-                                  AtomicSizeInBits / BitsPerByte);
+    return ConstantInt::get(IntegerType::get(ctx, SizeTBits),
+                            AtomicSizeInBits / BitsPerByte);
   }
 
-  std::pair<llvm::Value *, llvm::Value *> EmitAtomicCompareExchangeLibcall(
-      llvm::Value *ExpectedVal, llvm::Value *DesiredVal,
-      llvm::AtomicOrdering Success, llvm::AtomicOrdering Failure) {
-    LLVMContext &ctx = getLLVMContext();
-
-    // __atomic_compare_exchange's expected and desired are passed by pointers
-    // FIXME: types
-
-    // TODO: Get from llvm::TargetMachine / clang::TargetInfo
-    // 	if clang shares this codegen in future
-    constexpr uint64_t IntBits = 32;
-
-    // bool __atomic_compare_exchange(size_t size, void *obj, void *expected,
-    // 	void *desired, int success, int failure);
-    llvm::Value *Args[6] = {
-        getAtomicSizeValue(),
-        getAtomicPointer(),
-        ExpectedVal,
-        DesiredVal,
-        llvm::Constant::getIntegerValue(
-            llvm::IntegerType::get(ctx, IntBits),
-            llvm::APInt(IntBits, static_cast<uint64_t>(Success),
-                        /*signed=*/true)),
-        llvm::Constant::getIntegerValue(
-            llvm::IntegerType::get(ctx, IntBits),
-            llvm::APInt(IntBits, static_cast<uint64_t>(Failure),
-                        /*signed=*/true)),
-    };
-    auto Result = EmitAtomicLibcall(Builder, "__atomic_compare_exchange",
-                                    llvm::IntegerType::getInt1Ty(ctx), Args);
-    return std::make_pair(ExpectedVal, Result);
-  }
+  std::pair<Value *, Value *>
+  EmitAtomicCompareExchangeLibcall(Value *ExpectedVal, Value *DesiredVal,
+                                   AtomicOrdering Success,
+                                   AtomicOrdering Failure);
 
   Value *castToAtomicIntPointer(Value *addr) const {
     return addr; // opaque pointer
@@ -155,77 +94,17 @@ template <typename IRBuilderTy> struct AtomicInfo {
     return castToAtomicIntPointer(getAtomicPointer());
   }
 
-  std::pair<llvm::Value *, llvm::Value *>
-  EmitAtomicCompareExchangeOp(llvm::Value *ExpectedVal, llvm::Value *DesiredVal,
-                              llvm::AtomicOrdering Success,
-                              llvm::AtomicOrdering Failure,
-                              bool IsVolatile = false, bool IsWeak = false) {
-    // Do the atomic store.
-    Value *Addr = getAtomicAddressAsAtomicIntPointer();
-    auto *Inst = Builder->CreateAtomicCmpXchg(Addr, ExpectedVal, DesiredVal,
-                                              getAtomicAlignment(), Success,
-                                              Failure, llvm::SyncScope::System);
-    // Other decoration.
-    Inst->setVolatile(IsVolatile);
-    Inst->setWeak(IsWeak);
-
-    auto *PreviousVal = Builder->CreateExtractValue(Inst, /*Idxs=*/0);
-    auto *SuccessFailureVal = Builder->CreateExtractValue(Inst, /*Idxs=*/1);
-    return std::make_pair(PreviousVal, SuccessFailureVal);
-  }
+  std::pair<Value *, Value *>
+  EmitAtomicCompareExchangeOp(Value *ExpectedVal, Value *DesiredVal,
+                              AtomicOrdering Success, AtomicOrdering Failure,
+                              bool IsVolatile = false, bool IsWeak = false);
 
-  std::pair<llvm::Value *, llvm::Value *>
-  EmitAtomicCompareExchange(llvm::Value *ExpectedVal, llvm::Value *DesiredVal,
-                            llvm::AtomicOrdering Success,
-                            llvm::AtomicOrdering Failure, bool IsVolatile,
-                            bool IsWeak) {
-    if (shouldUseLibcall())
-      return EmitAtomicCompareExchangeLibcall(ExpectedVal, DesiredVal, Success,
-                                              Failure);
-
-    auto Res = EmitAtomicCompareExchangeOp(ExpectedVal, DesiredVal, Success,
-                                           Failure, IsVolatile, IsWeak);
-    return Res;
-  }
+  std::pair<Value *, Value *>
+  EmitAtomicCompareExchange(Value *ExpectedVal, Value *DesiredVal,
+                            AtomicOrdering Success, AtomicOrdering Failure,
+                            bool IsVolatile, bool IsWeak);
 
-  // void __atomic_load(size_t size, void *mem, void *return, int order);
-  std::pair<llvm::LoadInst *, llvm::AllocaInst *>
-  EmitAtomicLoadLibcall(llvm::AtomicOrdering AO) {
-    LLVMContext &Ctx = getLLVMContext();
-    Type *SizedIntTy = Type::getIntNTy(Ctx, getAtomicSizeInBits());
-    Type *ResultTy;
-    SmallVector<Value *, 6> Args;
-    AttributeList Attr;
-    Module *M = Builder->GetInsertBlock()->getModule();
-    const DataLayout &DL = M->getDataLayout();
-    Args.push_back(ConstantInt::get(DL.getIntPtrType(Ctx),
-                                    this->getAtomicSizeInBits() / 8));
-
-    Value *PtrVal = getAtomicPointer();
-    PtrVal = Builder->CreateAddrSpaceCast(PtrVal, PointerType::getUnqual(Ctx));
-    Args.push_back(PtrVal);
-    AllocaInst *AllocaResult =
-        CreateAlloca(Ty, getAtomicPointer()->getName() + "atomic.temp.load");
-    const Align AllocaAlignment = DL.getPrefTypeAlign(SizedIntTy);
-    AllocaResult->setAlignment(AllocaAlignment);
-    Args.push_back(AllocaResult);
-    Constant *OrderingVal =
-        ConstantInt::get(Type::getInt32Ty(Ctx), (int)toCABI(AO));
-    Args.push_back(OrderingVal);
-
-    ResultTy = Type::getVoidTy(Ctx);
-    SmallVector<Type *, 6> ArgTys;
-    for (Value *Arg : Args)
-      ArgTys.push_back(Arg->getType());
-    FunctionType *FnType = FunctionType::get(ResultTy, ArgTys, false);
-    FunctionCallee LibcallFn =
-        M->getOrInsertFunction("__atomic_load", FnType, Attr);
-    CallInst *Call = Builder->CreateCall(LibcallFn, Args);
-    Call->setAttributes(Attr);
-    return std::make_pair(
-        Builder->CreateAlignedLoad(Ty, AllocaResult, AllocaAlignment),
-        AllocaResult);
-  }
+  std::pair<LoadInst *, AllocaInst *> EmitAtomicLoadLibcall(AtomicOrdering AO);
 };
 } // end namespace llvm
 
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
index 1f0b129f867ae6..b6209cbc2e90af 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -480,16 +480,16 @@ class OpenMPIRBuilder {
         T(Triple(M.getTargetTriple())) {}
   ~OpenMPIRBuilder();
 
-  class AtomicInfo : public llvm::AtomicInfo<IRBuilder<>> {
+  class AtomicInfoBase : public llvm::AtomicInfo {
     llvm::Value *AtomicVar;
 
   public:
-    AtomicInfo(IRBuilder<> *Builder, llvm::Type *Ty, uint64_t AtomicSizeInBits,
-               uint64_t ValueSizeInBits, llvm::Align AtomicAlign,
-               llvm::Align ValueAlign, bool UseLibcall, llvm::Value *AtomicVar)
-        : llvm::AtomicInfo<IRBuilder<>>(Builder, Ty, AtomicSizeInBits,
-                                        ValueSizeInBits, AtomicAlign,
-                                        ValueAlign, UseLibcall),
+    AtomicInfoBase(IRBuilder<> *Builder, llvm::Type *Ty,
+                   uint64_t AtomicSizeInBits, uint64_t ValueSizeInBits,
+                   llvm::Align AtomicAlign, llvm::Align ValueAlign,
+                   bool UseLibcall, llvm::Value *AtomicVar)
+        : llvm::AtomicInfo(Builder, Ty, AtomicSizeInBits, ValueSizeInBits,
+                           AtomicAlign, ValueAlign, UseLibcall),
           AtomicVar(AtomicVar) {}
 
     llvm::Value *getAtomicPointer() const override { return AtomicVar; }
@@ -3095,15 +3095,6 @@ class OpenMPIRBuilder {
                    AtomicUpdateCallbackTy &UpdateOp, bool VolatileX,
                    bool IsXBinopExpr);
 
-  std::pair<llvm::LoadInst *, llvm::AllocaInst *>
-  EmitAtomicLoadLibcall(Value *X, Type *XElemTy, llvm::AtomicOrdering AO,
-                        uint64_t AtomicSizeInBits);
-
-  std::pair<llvm::Value *, llvm::Value *> EmitAtomicCompareExchangeLibcall(
-      Value *X, Type *XElemTy, uint64_t AtomicSizeInBits,
-      llvm::Value *ExpectedVal, llvm::Value *DesiredVal,
-      llvm::AtomicOrdering Success, llvm::AtomicOrdering Failure);
-
   /// Emit the binary op. described by \p RMWOp, using \p Src1 and \p Src2 .
   ///
   /// \Return The instruction
diff --git a/llvm/lib/Frontend/Atomic/Atomic.cpp b/llvm/lib/Frontend/Atomic/Atomic.cpp
index b54312293f9b06..89e7a139b8aeed 100644
--- a/llvm/lib/Frontend/Atomic/Atomic.cpp
+++ b/llvm/lib/Frontend/Atomic/Atomic.cpp
@@ -8,7 +8,147 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Frontend/Atomic/Atomic.h"
+#include "llvm/IR/IRBuilder.h"
 
-namespace {} // namespace
+using namespace llvm;
 
-namespace llvm {} // end namespace llvm
+bool AtomicInfo::shouldCastToInt(Type *ValTy, bool CmpXchg) {
+  if (ValTy->isFloatingPointTy())
+    return ValTy->isX86_FP80Ty() || CmpXchg;
+  return !ValTy->isIntegerTy() && !ValTy->isPointerTy();
+}
+
+Value *AtomicInfo::EmitAtomicLoadOp(AtomicOrdering AO, bool IsVolatile,
+                                    bool CmpXchg) {
+  Value *Ptr = getAtomicPointer();
+  Type *AtomicTy = Ty;
+  if (shouldCastToInt(Ty, CmpXchg))
+    AtomicTy = IntegerType::get(getLLVMContext(), AtomicSizeInBits);
+  LoadInst *Load =
+      Builder->CreateAlignedLoad(AtomicTy, Ptr, AtomicAlign, "atomic-load");
+  Load->setAtomic(AO);
+  if (IsVolatile)
+    Load->setVolatile(true);
+  decorateWithTBAA(Load);
+  return Load;
+}
+
+CallInst *AtomicInfo::EmitAtomicLibcall(IRBuilderBase *Builder,
+                                        StringRef fnName, Type *ResultType,
+                                        ArrayRef<Value *> Args) {
+  LLVMContext &ctx = Builder->getContext();
+  SmallVector<Type *, 6> ArgTys;
+  for (Value *Arg : Args)
+    ArgTys.push_back(Arg->getType());
+  FunctionType *FnType = FunctionType::get(ResultType, ArgTys, false);
+  Module *M = Builder->GetInsertBlock()->getModule();
+
+  // TODO: Use llvm::TargetLowering for Libcall ABI
+  AttrBuilder fnAttrBuilder(ctx);
+  fnAttrBuilder.addAttribute(Attribute::NoUnwind);
+  fnAttrBuilder.addAttribute(Attribute::WillReturn);
+  AttributeList fnAttrs =
+      AttributeList::get(ctx, AttributeList::FunctionIndex, fnAttrBuilder);
+  FunctionCallee LibcallFn = M->getOrInsertFunction(fnName, FnType, fnAttrs);
+  CallInst *Call = Builder->CreateCall(LibcallFn, Args);
+  return Call;
+}
+
+std::pair<Value *, Value *> AtomicInfo::EmitAtomicCompareExchangeLibcall(
+    Value *ExpectedVal, Value *DesiredVal, AtomicOrdering Success,
+    AtomicOrdering Failure) {
+  LLVMContext &ctx = getLLVMContext();
+
+  // __atomic_compare_exchange's expected and desired are passed by pointers
+  // FIXME: types
+
+  // TODO: Get from llvm::TargetMachine / clang::TargetInfo
+  // if clang shares this codegen in future
+  constexpr uint64_t IntBits = 32;
+
+  // bool __atomic_compare_exchange(size_t size, void *obj, void *expected,
+  //  void *desired, int success, int failure);
+
+  Value *Args[6] = {
+      getAtomicSizeValue(),
+      getAtomicPointer(),
+      ExpectedVal,
+      DesiredVal,
+      Constant::getIntegerValue(IntegerType::get(ctx, IntBits),
+                                APInt(IntBits, static_cast<uint64_t>(Success),
+                                      /*signed=*/true)),
+      Constant::getIntegerValue(IntegerType::get(ctx, IntBits),
+                                APInt(IntBits, static_cast<uint64_t>(Failure),
+                                      /*signed=*/true)),
+  };
+  auto Result = EmitAtomicLibcall(Builder, "__atomic_compare_exchange",
+                                  IntegerType::getInt1Ty(ctx), Args);
+  return std::make_pair(ExpectedVal, Result);
+}
+
+std::pair<Value *, Value *> AtomicInfo::EmitAtomicCompareExchangeOp(
+    Value *ExpectedVal, Value *DesiredVal, AtomicOrdering Success,
+    AtomicOrdering Failure, bool IsVolatile, bool IsWeak) {
+  // Do the atomic store.
+  Value *Addr = getAtomicAddressAsAtomicIntPointer();
+  auto *Inst = Builder->CreateAtomicCmpXchg(Addr, ExpectedVal, DesiredVal,
+                                            getAtomicAlignment(), Success,
+                                            Failure, SyncScope::System);
+
+  // Other decoration.
+  Inst->setVolatile(IsVolatile);
+  Inst->setWeak(IsWeak);
+  auto *PreviousVal = Builder->CreateExtractValue(Inst, /*Idxs=*/0);
+  auto *SuccessFailureVal = Builder->CreateExtractValue(Inst, /*Idxs=*/1);
+  return std::make_pair(PreviousVal, SuccessFailureVal);
+}
+
+std::pair<LoadInst *, AllocaInst *>
+AtomicInfo::EmitAtomicLoadLibcall(AtomicOrdering AO) {
+  LLVMContext &Ctx = getLLVMContext();
+  Type *SizedIntTy = Type::getIntNTy(Ctx, getAtomicSizeInBits());
+  Type *ResultTy;
+  SmallVector<Value *, 6> Args;
+  AttributeList Attr;
+  Module *M = Builder->GetInsertBlock()->getModule();
+  const DataLayout &DL = M->getDataLayout();
+  Args.push_back(
+      ConstantInt::get(DL.getIntPtrType(Ctx), this->getAtomicSizeInBits() / 8));
+
+  Value *PtrVal = getAtomicPointer();
+  PtrVal = Builder->CreateAddrSpaceCast(PtrVal, PointerType::getUnqual(Ctx));
+  Args.push_back(PtrVal);
+  AllocaInst *AllocaResult =
+      CreateAlloca(Ty, getAtomicPointer()->getName() + "atomic.temp.load");
+  const Align AllocaAlignment = DL.getPrefTypeAlign(SizedIntTy);
+  AllocaResult->setAlignment(AllocaAlignment);
+  Args.push_back(AllocaResult);
+  Constant *OrderingVal =
+      ConstantInt::get(Type::getInt32Ty(Ctx), (int)toCABI(AO));
+  Args.push_back(OrderingVal);
+
+  ResultTy = Type::getVoidTy(Ctx);
+  SmallVector<Type *, 6> ArgTys;
+  for (Value *Arg : Args)
+    ArgTys.push_back(Arg->getType());
+  FunctionType *FnType = FunctionType::get(ResultTy, ArgTys, false);
+  FunctionCallee LibcallFn =
+      M->getOrInsertFunction("__atomic_load", FnType, Attr);
+  CallInst *Call = Builder->CreateCall(LibcallFn, Args);
+  Call->setAttributes(Attr);
+  return std::make_pair(
+      Builder->CreateAlignedLoad(Ty, AllocaResult, AllocaAlignment),
+      AllocaResult);
+}
+
+std::pair<Value *, Value *> AtomicInfo::EmitAtomicCompareExchange(
+    Value *ExpectedVal, Value *DesiredVal, AtomicOrdering Success,
+    AtomicOrdering Failure, bool IsVolatile, bool IsWeak) {
+  if (shouldUseLibcall())
+    return EmitAtomicCompareExchangeLibcall(ExpectedVal, DesiredVal, Success,
+                                            Failure);
+
+  auto Res = EmitAtomicCompareExchangeOp(ExpectedVal, DesiredVal, Success,
+                                         Failure, IsVolatile, IsWeak);
+  return Res;
+}
diff --git a/llvm/lib/Frontend/OpenMP/CMakeLists.txt b/llvm/lib/Frontend/OpenMP/CMakeLists.txt
index 82d2a9ae7c5335..35c607866a94eb 100644
--- a/llvm/lib/Frontend/OpenMP/CMakeLists.txt
+++ b/llvm/lib/Frontend/OpenMP/CMakeLists.txt
@@ -22,4 +22,5 @@ add_llvm_component_library(LLVMFrontendOpenMP
   Scalar
   BitReader
   FrontendOffloading
+  FrontendAtomic
   )
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 21004e6a15d495..58d178b8b0c3a4 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -8073,7 +8073,7 @@ OpenMPIRBuilder::createAtomicRead(const LocationDescription &Loc,
     const DataLayout &LoadDL = OldVal->getModule()->getDataLayout();
     unsigned LoadSize =
         LoadDL.getTypeStoreSize(OldVal->getPointerOperand()->getType());
-    OpenMPIRBuilder::AtomicInfo atomicInfo(
+    OpenMPIRBuilder::AtomicInfoBase atomicInfo(
         &Builder, XElemTy, LoadSize * 8, LoadSize * 8, OldVal->getAlign(),
         O...
[truncated]

Copy link

github-actions bot commented Dec 9, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for following up on this!

CallInst *Call = Builder->CreateCall(LibcallFn, Args);
return Call;
}
CallInst *EmitAtomicLibcall(IRBuilderBase *Builder, StringRef fnName,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need the Builder argument if it's already part of the class?

#include "llvm/IR/Instructions.h"
#include "llvm/IR/Intrinsics.h"
#include "llvm/IR/Module.h"
#include "llvm/IR/Operator.h"
#include "llvm/IR/RuntimeLibcalls.h"

namespace llvm {
class AtomicInfo;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant forward declare?

@@ -12,44 +12,45 @@

#include "llvm/ADT/DenseMap.h"
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/IRBuilder.h"
#include "llvm/IR/Instructions.h"
#include "llvm/IR/Intrinsics.h"
#include "llvm/IR/Module.h"
#include "llvm/IR/Operator.h"
#include "llvm/IR/RuntimeLibcalls.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe double check that you still need these includes in the header? E.g. I don't think you need RuntimeLibcalls.h?

@kparzysz
Copy link
Contributor

Please fix the comments at the top of both files. Both say "Atomic.h", and the first line is broken into two.

I think that too many includes are now removed. For example, the header file uses IntegerType::get, but there is no #include "llvm/IR/DerivedTypes.h", or #include <utility> for std::pair. I believe the preference is to IWYU, but I'll defer to @nikic to decide.

@NimishMishra
Copy link
Contributor Author

Please fix the comments at the top of both files. Both say "Atomic.h", and the first line is broken into two.

I think that too many includes are now removed. For example, the header file uses IntegerType::get, but there is no #include "llvm/IR/DerivedTypes.h", or #include <utility> for std::pair. I believe the preference is to IWYU, but I'll defer to @nikic to decide.

Thanks. I have made the changes.

@NimishMishra NimishMishra merged commit 24fc8f0 into llvm:main Dec 23, 2024
8 checks passed
chapuni added a commit that referenced this pull request Dec 23, 2024
@llvm-ci
Copy link
Collaborator

llvm-ci commented Dec 24, 2024

LLVM Buildbot has detected a new failure on builder lld-x86_64-win running on as-worker-93 while building llvm at step 7 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/1908

Here is the relevant piece of the build log for the reference
Step 7 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests.exe/38/87' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe-LLVM-Unit-7840-38-87.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=87 GTEST_SHARD_INDEX=38 C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe
--

Script:
--
C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe --gtest_filter=ProgramEnvTest.CreateProcessLongPath
--
C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp(160): error: Expected equality of these values:
  0
  RC
    Which is: -2

C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp(163): error: fs::remove(Twine(LongPath)): did not return errc::success.
error number: 13
error message: permission denied



C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp:160
Expected equality of these values:
  0
  RC
    Which is: -2

C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp:163
fs::remove(Twine(LongPath)): did not return errc::success.
error number: 13
error message: permission denied




********************


bherrera pushed a commit to misttech/mist-os that referenced this pull request Jan 6, 2025
llvm/llvm-project#119199 added
LLVMFrontendAtomic dependency for LLVMFrontendOpenMP.
This patch adds the new dependency through a soft
transition for the LLVM libraries.

Bug: 386808324
Change-Id: Ib886d73556b2d5e94bc655625bff4ddb7eaff238
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1180341
Fuchsia-Auto-Submit: Gulfem Savrun Yeniceri <[email protected]>
Commit-Queue: Auto-Submit <[email protected]>
Reviewed-by: Petr Hosek <[email protected]>
bherrera pushed a commit to misttech/integration that referenced this pull request Jan 6, 2025
llvm/llvm-project#119199 added
LLVMFrontendAtomic dependency for LLVMFrontendOpenMP.
This patch adds the new dependency through a soft
transition for the LLVM libraries.

Original-Bug: 386808324
Original-Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1180341
Original-Revision: c141ef4ff6b56bdf681ed16f3d05163567bc40f8
GitOrigin-RevId: b79b5308474907323e93835561d69ae20f99a532
Change-Id: Ie49b76fc74790151460ff1d19b1f0fd561622fa8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:openmp OpenMP related changes to Clang flang:openmp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants