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