tianshilei1992 created this revision.
tianshilei1992 added reviewers: jdoerfert, ABataev.
Herald added subscribers: mattd, gchakrabarti, asavonic, jholewinski.
Herald added a project: All.
tianshilei1992 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

With respect of `atomicrmw` instruction, different targets have different
backend support. The front end can always emit the intruction, but when it comes
to the backend, the compiler could crash at instruction selection because the
target doesn't support the instruction/operator at all. Currently we don't have
a way in the front end to tell if an `atomicrmw` with specific operator and type
is supported. This patch adds a virtual function `hasAtomicrmw` to the class
`TargetInfo` and it is expected to be implemented by all targets accordingly.
In this way, we can always try to emit `atomicrmw` first because it has better
performance than falling back to CAS loop. Currently at the PoC stage, I only 
make
it in NVPTX, but if the community is happy with this change, I'll add to all the
targets.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125350

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp

Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===================================================================
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -5925,6 +5925,11 @@
   case BO_Comma:
     llvm_unreachable("Unsupported atomic update operation");
   }
+
+  if (!Context.getTargetInfo().hasAtomicrmw(RMWOp,
+                                            X.getAddress(CGF).getElementType()))
+    return std::make_pair(false, RValue::get(nullptr));
+
   llvm::Value *UpdateVal = Update.getScalarVal();
   if (auto *IC = dyn_cast<llvm::ConstantInt>(UpdateVal)) {
     if (IsInteger)
Index: clang/lib/Basic/Targets/NVPTX.h
===================================================================
--- clang/lib/Basic/Targets/NVPTX.h
+++ clang/lib/Basic/Targets/NVPTX.h
@@ -17,6 +17,7 @@
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TargetOptions.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/IR/Instructions.h"
 #include "llvm/Support/Compiler.h"
 
 namespace clang {
@@ -176,6 +177,8 @@
   }
 
   bool hasBitIntType() const override { return true; }
+
+  bool hasAtomicrmw(llvm::AtomicRMWInst::BinOp, llvm::Type *) const override;
 };
 } // namespace targets
 } // namespace clang
Index: clang/lib/Basic/Targets/NVPTX.cpp
===================================================================
--- clang/lib/Basic/Targets/NVPTX.cpp
+++ clang/lib/Basic/Targets/NVPTX.cpp
@@ -273,3 +273,43 @@
   return llvm::makeArrayRef(BuiltinInfo, clang::NVPTX::LastTSBuiltin -
                                              Builtin::FirstTSBuiltin);
 }
+
+bool NVPTXTargetInfo::hasAtomicrmw(llvm::AtomicRMWInst::BinOp Op,
+                                   llvm::Type *Ty) const {
+  switch (Op) {
+  default:
+    llvm_unreachable("bad binop");
+  case llvm::AtomicRMWInst::BinOp::FSub:
+  case llvm::AtomicRMWInst::BinOp::Nand:
+    return false;
+  case llvm::AtomicRMWInst::BinOp::FAdd:
+    assert(Ty->isFloatingPointTy());
+    if (Ty->isFloatTy())
+      return true;
+    if (Ty->isDoubleTy() && GPU >= CudaArch::SM_60)
+      return true;
+    return false;
+  case llvm::AtomicRMWInst::BinOp::Add:
+  case llvm::AtomicRMWInst::BinOp::Sub:
+  case llvm::AtomicRMWInst::BinOp::Max:
+  case llvm::AtomicRMWInst::BinOp::Min:
+  case llvm::AtomicRMWInst::BinOp::UMax:
+  case llvm::AtomicRMWInst::BinOp::UMin:
+  case llvm::AtomicRMWInst::BinOp::Xchg:
+  case llvm::AtomicRMWInst::BinOp::And:
+  case llvm::AtomicRMWInst::BinOp::Or:
+  case llvm::AtomicRMWInst::BinOp::Xor:
+    assert(Ty->isIntegerTy());
+    switch (cast<llvm::IntegerType>(Ty)->getBitWidth()) {
+    case 32:
+      return true;
+    case 64:
+      return true;
+    default:
+      return false;
+    }
+    return false;
+  }
+
+  return false;
+}
Index: clang/include/clang/Basic/TargetInfo.h
===================================================================
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -31,6 +31,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Frontend/OpenMP/OMPGridValues.h"
+#include "llvm/IR/Instructions.h"
 #include "llvm/Support/DataTypes.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/VersionTuple.h"
@@ -756,6 +757,12 @@
             llvm::isPowerOf2_64(AtomicSizeInBits / getCharWidth()));
   }
 
+  /// Returns true if the given target supports atomicrmw with given operator
+  /// and type.
+  virtual bool hasAtomicrmw(llvm::AtomicRMWInst::BinOp, llvm::Type *) const {
+    llvm_unreachable("hasAtomicrmw not implemented on this target");
+  }
+
   /// Return the maximum vector alignment supported for the given target.
   unsigned getMaxVectorAlign() const { return MaxVectorAlign; }
   /// Return default simd alignment for the given target. Generally, this
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to