[LLVM] Add a check if should cast atomic operations to integer type

[tianshilei1992]

Summary

Currently for atomic load, store, and rmw instructions, as long as the
operand is floating-point value, they are casted to integer. Nowadays many
targets can actually support part of atomic operations with floating-point
operands. For example, NVPTX supports atomic load and store of floating-point
values. This patch adds a series interface functions shouldCastAtomicXXXInIR,
and the default implementations are same as what we currently do. Later for
targets can have their specialization.

[jdoerfert]

LG

[tra]

llvm/include/llvm/CodeGen/TargetLowering.h

2050

just cast. It's an irregular verb.

2052

Would it make sense to generalize all these into should{Cast,Expand}AtomicOpInIR(Instruction *I) so we don't have to encode each instruction variant into the function name?

[tianshilei1992]

rebase and fix comment

[tianshilei1992]

llvm/include/llvm/CodeGen/TargetLowering.h

2052

Sounds like a good idea. I'll do it in another patch.

[jyknight]

As somewhat coincidentally just discussed in the thread on https://reviews.llvm.org/D124728 the target hooks don't really make sense as implemented here.

Whether a cast to an integer is required is due in part to what code AtomicExpandPass itself generates (configured based upon existing target hooks), and whether _that_ expansion requires the value to be an integer (almost all of them currently do). Requiring targets to deduce those requirements and encode it into a brand new target hook seems error-prone, and should be avoided.

@efriedma

llvm/include/llvm/CodeGen/TargetLowering.h

public:
   };
 
   /// Enum that specifies what an atomic load/AtomicRMWInst is expanded
   /// to, if at all. Exists because different targets have different levels of
   /// support for these atomic instructions, and also have different options
   /// w.r.t. what they should expand to.
   enum class AtomicExpansionKind {
     None,    // Don't expand the instruction.
+    CastToInteger,    // Cast the atomic instruction to another type, e.g. from
+                      // floating-point to integer type.
     LLSC,    // Expand the instruction into loadlinked/storeconditional; used
              // by ARM/AArch64.
     LLOnly,  // Expand the (load) instruction into just a load-linked, which has
              // greater atomic guarantees than a normal load.
     CmpXChg, // Expand the instruction into cmpxchg; used by at least X86.
     MaskedIntrinsic,  // Use a target-specific intrinsic for the LL/SC loop.
     BitTestIntrinsic, // Use a target-specific intrinsic for special bit
                       // operations; used by X86.
public:
   }
 
   /// Returns how the given (atomic) load should be expanded by the
   /// IR-level AtomicExpand pass.
   virtual AtomicExpansionKind shouldExpandAtomicLoadInIR(LoadInst *LI) const {
     return AtomicExpansionKind::None;
   }
 
+  /// Returns how the given (atomic) load should be casted by the IR-level
+  /// AtomicExpand pass.
+  virtual AtomicExpansionKind shouldCastAtomicLoadInIR(LoadInst *LI) const {
+    if (LI->getType()->isFloatingPointTy())
+      return AtomicExpansionKind::CastToInteger;
+    return AtomicExpansionKind::None;
+  }
+
   /// Returns how the given (atomic) store should be expanded by the IR-level
   /// AtomicExpand pass into. For instance AtomicExpansionKind::Expand will try
   /// to use an atomicrmw xchg.
   virtual AtomicExpansionKind shouldExpandAtomicStoreInIR(StoreInst *SI) const {
     return AtomicExpansionKind::None;
   }
 
+  /// Returns how the given (atomic) store should be casted by the IR-level
+  /// AtomicExpand pass into. For instance AtomicExpansionKind::CastToInteger
+  /// will try to cast the operands to integer values.
+  virtual AtomicExpansionKind shouldCastAtomicStoreInIR(StoreInst *SI) const {
+    if (SI->getValueOperand()->getType()->isFloatingPointTy())
+      return AtomicExpansionKind::CastToInteger;
+    return AtomicExpansionKind::None;
+  }
+
   /// Returns how the given atomic cmpxchg should be expanded by the IR-level
   /// AtomicExpand pass.
   virtual AtomicExpansionKind
   shouldExpandAtomicCmpXchgInIR(AtomicCmpXchgInst *AI) const {
     return AtomicExpansionKind::None;
   }
 
   /// Returns how the IR-level AtomicExpand pass should expand the given
   /// AtomicRMW, if at all. Default is to never expand.
   virtual AtomicExpansionKind shouldExpandAtomicRMWInIR(AtomicRMWInst *RMW) const {
     return RMW->isFloatingPointOperation() ?
       AtomicExpansionKind::CmpXChg : AtomicExpansionKind::None;
   }
 
+  /// Returns how the given atomic atomicrmw should be casted by the IR-level
+  /// AtomicExpand pass.
+  virtual AtomicExpansionKind
+  shouldCastAtomicRMWIInIR(AtomicRMWInst *RMWI) const {
+    if (RMWI->getOperation() == AtomicRMWInst::Xchg &&
+        RMWI->getValOperand()->getType()->isFloatingPointTy())
+      return AtomicExpansionKind::CastToInteger;
+
+    return AtomicExpansionKind::None;
+  }
+
   /// On some platforms, an AtomicRMW that never actually modifies the value
   /// (such as fetch_add of 0) can be turned into a fence followed by an
   /// atomic load. This may sound useless, but it makes it possible for the
   /// processor to keep the cacheline shared, dramatically improving
   /// performance. And such idempotent RMWs are useful for implementing some
   /// kinds of locks, see for example (justification + benchmarks):
   /// http://www.hpl.hp.com/techreports/2012/HPL-2012-68.pdf
   /// This method tries doing that transformation, returning the atomic load if

llvm/lib/CodeGen/AtomicExpandPass.cpp

if (TLI->shouldInsertFencesForAtomic(I)) {
       }
 
       if (FenceOrdering != AtomicOrdering::Monotonic) {
         MadeChange |= bracketInstWithFences(I, FenceOrdering);
       }
     }
 
     if (LI) {
-      if (LI->getType()->isFloatingPointTy()) {
+      if (TLI->shouldCastAtomicLoadInIR(LI) ==
+          TargetLoweringBase::AtomicExpansionKind::CastToInteger) {
         // TODO: add a TLI hook to control this so that each target can
         // convert to lowering the original type one at a time.
         LI = convertAtomicLoadToIntegerType(LI);
         assert(LI->getType()->isIntegerTy() && "invariant broken");
         MadeChange = true;
       }
 
       MadeChange |= tryExpandAtomicLoad(LI);
     } else if (SI) {
-      if (SI->getValueOperand()->getType()->isFloatingPointTy()) {
+      if (TLI->shouldCastAtomicStoreInIR(SI) ==
+          TargetLoweringBase::AtomicExpansionKind::CastToInteger) {
         // TODO: add a TLI hook to control this so that each target can
         // convert to lowering the original type one at a time.
         SI = convertAtomicStoreToIntegerType(SI);
         assert(SI->getValueOperand()->getType()->isIntegerTy() &&
                "invariant broken");
         MadeChange = true;
       }
 
       if (tryExpandAtomicStore(SI))
         MadeChange = true;
     } else if (RMWI) {
       // There are two different ways of expanding RMW instructions:
       // - into a load if it is idempotent
       // - into a Cmpxchg/LL-SC loop otherwise
       // we try them in that order.
 
       if (isIdempotentRMW(RMWI) && simplifyIdempotentRMW(RMWI)) {
         MadeChange = true;
       } else {
         AtomicRMWInst::BinOp Op = RMWI->getOperation();
-        if (Op == AtomicRMWInst::Xchg &&
-            RMWI->getValOperand()->getType()->isFloatingPointTy()) {
+        if (TLI->shouldCastAtomicRMWIInIR(RMWI) ==
+            TargetLoweringBase::AtomicExpansionKind::CastToInteger) {
           // TODO: add a TLI hook to control this so that each target can
           // convert to lowering the original type one at a time.
           RMWI = convertAtomicXchgToIntegerType(RMWI);
           assert(RMWI->getValOperand()->getType()->isIntegerTy() &&
                  "invariant broken");
           MadeChange = true;
         }
         unsigned MinCASSize = TLI->getMinCmpXchgSizeInBits() / 8;