Skip to content
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

AMDGPU: Do not bitcast atomic load in IR #90060

Merged
merged 2 commits into from
Apr 26, 2024

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Apr 25, 2024

These hooks should be removed. This is a trivial legalization transform the legalizer needs to support. The IR just complicates things, and it was losing metadata. Implement the DAG promotion support, and switch AMDGPU over to using it.

Really we'd be a lot better off merging ATOMIC_LOAD and LOAD like GlobalISel does.

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 25, 2024

@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-llvm-selectiondag

Author: Matt Arsenault (arsenm)

Changes

These hooks should be removed. This is a trivial legalization transform the legalizer needs to support. The IR just complicates things, and it was losing metadata. Implement the DAG promotion support, and switch AMDGPU over to using it.

Really we'd be a lot better off merging ATOMIC_LOAD and LOAD like GlobalISel does.


Full diff: https://github.com/llvm/llvm-project/pull/90060.diff

6 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp (+15)
  • (modified) llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp (+39)
  • (modified) llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h (+2)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp (+13)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h (+6)
  • (modified) llvm/test/Transforms/AtomicExpand/AMDGPU/no-expand-atomic-load.ll (+20-34)
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
index 24f69ea1b742a6..9c073aa51324f8 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
@@ -5575,6 +5575,21 @@ void SelectionDAGLegalize::PromoteNode(SDNode *Node) {
     Results.push_back(NewAtomic.getValue(1));
     break;
   }
+  case ISD::ATOMIC_LOAD: {
+    AtomicSDNode *AM = cast<AtomicSDNode>(Node);
+    SDLoc SL(Node);
+    assert(NVT.getSizeInBits() == OVT.getSizeInBits() &&
+           "unexpected promotion type");
+    assert(AM->getMemoryVT().getSizeInBits() == NVT.getSizeInBits() &&
+           "unexpected atomic_load with illegal type");
+
+    SDValue NewAtomic =
+        DAG.getAtomic(ISD::ATOMIC_LOAD, SL, NVT, DAG.getVTList(NVT, MVT::Other),
+                      {AM->getChain(), AM->getBasePtr()}, AM->getMemOperand());
+    Results.push_back(DAG.getNode(ISD::BITCAST, SL, OVT, NewAtomic));
+    Results.push_back(NewAtomic.getValue(1));
+    break;
+  }
   case ISD::SPLAT_VECTOR: {
     SDValue Scalar = Node->getOperand(0);
     MVT ScalarType = Scalar.getSimpleValueType();
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
index 7685bc73cf9652..abe5be76382556 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
@@ -2449,6 +2449,9 @@ void DAGTypeLegalizer::PromoteFloatResult(SDNode *N, unsigned ResNo) {
       R = PromoteFloatRes_STRICT_FP_ROUND(N);
       break;
     case ISD::LOAD:       R = PromoteFloatRes_LOAD(N); break;
+    case ISD::ATOMIC_LOAD:
+      R = PromoteFloatRes_ATOMIC_LOAD(N);
+      break;
     case ISD::SELECT:     R = PromoteFloatRes_SELECT(N); break;
     case ISD::SELECT_CC:  R = PromoteFloatRes_SELECT_CC(N); break;
 
@@ -2695,6 +2698,25 @@ SDValue DAGTypeLegalizer::PromoteFloatRes_LOAD(SDNode *N) {
   return DAG.getNode(GetPromotionOpcode(VT, NVT), SDLoc(N), NVT, newL);
 }
 
+SDValue DAGTypeLegalizer::PromoteFloatRes_ATOMIC_LOAD(SDNode *N) {
+  AtomicSDNode *AM = cast<AtomicSDNode>(N);
+  EVT VT = AM->getValueType(0);
+
+  // Load the value as an integer value with the same number of bits.
+  EVT IVT = EVT::getIntegerVT(*DAG.getContext(), VT.getSizeInBits());
+  SDValue newL = DAG.getAtomic(
+      ISD::ATOMIC_LOAD, SDLoc(N), IVT, DAG.getVTList(IVT, MVT::Other),
+      {AM->getChain(), AM->getBasePtr()}, AM->getMemOperand());
+
+  // Legalize the chain result by replacing uses of the old value chain with the
+  // new one
+  ReplaceValueWith(SDValue(N, 1), newL.getValue(1));
+
+  // Convert the integer value to the desired FP type
+  EVT NVT = TLI.getTypeToTransformTo(*DAG.getContext(), VT);
+  return DAG.getNode(GetPromotionOpcode(VT, IVT), SDLoc(N), NVT, newL);
+}
+
 // Construct a new SELECT node with the promoted true- and false- values.
 SDValue DAGTypeLegalizer::PromoteFloatRes_SELECT(SDNode *N) {
   SDValue TrueVal = GetPromotedFloat(N->getOperand(1));
@@ -2855,6 +2877,9 @@ void DAGTypeLegalizer::SoftPromoteHalfResult(SDNode *N, unsigned ResNo) {
   case ISD::FFREXP:      R = SoftPromoteHalfRes_FFREXP(N); break;
 
   case ISD::LOAD:        R = SoftPromoteHalfRes_LOAD(N); break;
+  case ISD::ATOMIC_LOAD:
+    R = SoftPromoteHalfRes_ATOMIC_LOAD(N);
+    break;
   case ISD::SELECT:      R = SoftPromoteHalfRes_SELECT(N); break;
   case ISD::SELECT_CC:   R = SoftPromoteHalfRes_SELECT_CC(N); break;
   case ISD::SINT_TO_FP:
@@ -3039,6 +3064,20 @@ SDValue DAGTypeLegalizer::SoftPromoteHalfRes_LOAD(SDNode *N) {
   return NewL;
 }
 
+SDValue DAGTypeLegalizer::SoftPromoteHalfRes_ATOMIC_LOAD(SDNode *N) {
+  AtomicSDNode *AM = cast<AtomicSDNode>(N);
+
+  // Load the value as an integer value with the same number of bits.
+  SDValue NewL = DAG.getAtomic(
+      ISD::ATOMIC_LOAD, SDLoc(N), MVT::i16, DAG.getVTList(MVT::i16, MVT::Other),
+      {AM->getChain(), AM->getBasePtr()}, AM->getMemOperand());
+
+  // Legalize the chain result by replacing uses of the old value chain with the
+  // new one
+  ReplaceValueWith(SDValue(N, 1), NewL.getValue(1));
+  return NewL;
+}
+
 SDValue DAGTypeLegalizer::SoftPromoteHalfRes_SELECT(SDNode *N) {
   SDValue Op1 = GetSoftPromotedHalf(N->getOperand(1));
   SDValue Op2 = GetSoftPromotedHalf(N->getOperand(2));
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
index 9c855e55855312..4a2c7b355eb528 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
@@ -691,6 +691,7 @@ class LLVM_LIBRARY_VISIBILITY DAGTypeLegalizer {
   SDValue PromoteFloatRes_FP_ROUND(SDNode *N);
   SDValue PromoteFloatRes_STRICT_FP_ROUND(SDNode *N);
   SDValue PromoteFloatRes_LOAD(SDNode *N);
+  SDValue PromoteFloatRes_ATOMIC_LOAD(SDNode *N);
   SDValue PromoteFloatRes_SELECT(SDNode *N);
   SDValue PromoteFloatRes_SELECT_CC(SDNode *N);
   SDValue PromoteFloatRes_UnaryOp(SDNode *N);
@@ -734,6 +735,7 @@ class LLVM_LIBRARY_VISIBILITY DAGTypeLegalizer {
   SDValue SoftPromoteHalfRes_FFREXP(SDNode *N);
   SDValue SoftPromoteHalfRes_FP_ROUND(SDNode *N);
   SDValue SoftPromoteHalfRes_LOAD(SDNode *N);
+  SDValue SoftPromoteHalfRes_ATOMIC_LOAD(SDNode *N);
   SDValue SoftPromoteHalfRes_SELECT(SDNode *N);
   SDValue SoftPromoteHalfRes_SELECT_CC(SDNode *N);
   SDValue SoftPromoteHalfRes_UnaryOp(SDNode *N);
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp b/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
index f4a747784d1fd2..7993b63121110c 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
@@ -148,6 +148,19 @@ AMDGPUTargetLowering::AMDGPUTargetLowering(const TargetMachine &TM,
   setOperationAction(ISD::LOAD, MVT::i128, Promote);
   AddPromotedToType(ISD::LOAD, MVT::i128, MVT::v4i32);
 
+  // TODO: Would be better to consume as directly legal
+  setOperationAction(ISD::ATOMIC_LOAD, MVT::f32, Promote);
+  AddPromotedToType(ISD::ATOMIC_LOAD, MVT::f32, MVT::i32);
+
+  setOperationAction(ISD::ATOMIC_LOAD, MVT::f64, Promote);
+  AddPromotedToType(ISD::ATOMIC_LOAD, MVT::f64, MVT::i64);
+
+  setOperationAction(ISD::ATOMIC_LOAD, MVT::f16, Promote);
+  AddPromotedToType(ISD::ATOMIC_LOAD, MVT::f16, MVT::i16);
+
+  setOperationAction(ISD::ATOMIC_LOAD, MVT::bf16, Promote);
+  AddPromotedToType(ISD::ATOMIC_LOAD, MVT::bf16, MVT::i16);
+
   // There are no 64-bit extloads. These should be done as a 32-bit extload and
   // an extension to 64-bit.
   for (MVT VT : MVT::integer_valuetypes())
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h b/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
index 72661a8d29f816..269c414521dbcc 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
@@ -230,6 +230,12 @@ class AMDGPUTargetLowering : public TargetLowering {
   bool isCheapToSpeculateCtlz(Type *Ty) const override;
 
   bool isSDNodeAlwaysUniform(const SDNode *N) const override;
+
+  // FIXME: This hook should not exist
+  AtomicExpansionKind shouldCastAtomicLoadInIR(LoadInst *LI) const override {
+    return AtomicExpansionKind::None;
+  }
+
   static CCAssignFn *CCAssignFnForCall(CallingConv::ID CC, bool IsVarArg);
   static CCAssignFn *CCAssignFnForReturn(CallingConv::ID CC, bool IsVarArg);
 
diff --git a/llvm/test/Transforms/AtomicExpand/AMDGPU/no-expand-atomic-load.ll b/llvm/test/Transforms/AtomicExpand/AMDGPU/no-expand-atomic-load.ll
index fd5a2044db48f3..b1497aefe9b93e 100644
--- a/llvm/test/Transforms/AtomicExpand/AMDGPU/no-expand-atomic-load.ll
+++ b/llvm/test/Transforms/AtomicExpand/AMDGPU/no-expand-atomic-load.ll
@@ -6,8 +6,7 @@
 define float @load_atomic_f32_global_system(ptr addrspace(1) %ptr) {
 ; CHECK-LABEL: define float @load_atomic_f32_global_system(
 ; CHECK-SAME: ptr addrspace(1) [[PTR:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = load atomic i32, ptr addrspace(1) [[PTR]] seq_cst, align 4
-; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i32 [[TMP1]] to float
+; CHECK-NEXT:    [[TMP2:%.*]] = load atomic float, ptr addrspace(1) [[PTR]] seq_cst, align 4, !some.unknown.md [[META0:![0-9]+]]
 ; CHECK-NEXT:    ret float [[TMP2]]
 ;
   %ld = load atomic float, ptr addrspace(1) %ptr seq_cst, align 4, !some.unknown.md !0
@@ -17,8 +16,7 @@ define float @load_atomic_f32_global_system(ptr addrspace(1) %ptr) {
 define float @load_atomic_f32_global_agent(ptr addrspace(1) %ptr) {
 ; CHECK-LABEL: define float @load_atomic_f32_global_agent(
 ; CHECK-SAME: ptr addrspace(1) [[PTR:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = load atomic i32, ptr addrspace(1) [[PTR]] syncscope("agent") seq_cst, align 4
-; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i32 [[TMP1]] to float
+; CHECK-NEXT:    [[TMP2:%.*]] = load atomic float, ptr addrspace(1) [[PTR]] syncscope("agent") seq_cst, align 4, !some.unknown.md [[META0]]
 ; CHECK-NEXT:    ret float [[TMP2]]
 ;
   %ld = load atomic float, ptr addrspace(1) %ptr syncscope("agent") seq_cst, align 4, !some.unknown.md !0
@@ -28,8 +26,7 @@ define float @load_atomic_f32_global_agent(ptr addrspace(1) %ptr) {
 define float @load_atomic_f32_local(ptr addrspace(3) %ptr) {
 ; CHECK-LABEL: define float @load_atomic_f32_local(
 ; CHECK-SAME: ptr addrspace(3) [[PTR:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = load atomic i32, ptr addrspace(3) [[PTR]] seq_cst, align 4
-; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i32 [[TMP1]] to float
+; CHECK-NEXT:    [[TMP2:%.*]] = load atomic float, ptr addrspace(3) [[PTR]] seq_cst, align 4, !some.unknown.md [[META0]]
 ; CHECK-NEXT:    ret float [[TMP2]]
 ;
   %ld = load atomic float, ptr addrspace(3) %ptr seq_cst, align 4, !some.unknown.md !0
@@ -39,8 +36,7 @@ define float @load_atomic_f32_local(ptr addrspace(3) %ptr) {
 define float @load_atomic_f32_flat_system(ptr %ptr) {
 ; CHECK-LABEL: define float @load_atomic_f32_flat_system(
 ; CHECK-SAME: ptr [[PTR:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = load atomic i32, ptr [[PTR]] seq_cst, align 4
-; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i32 [[TMP1]] to float
+; CHECK-NEXT:    [[TMP2:%.*]] = load atomic float, ptr [[PTR]] seq_cst, align 4, !some.unknown.md [[META0]]
 ; CHECK-NEXT:    ret float [[TMP2]]
 ;
   %ld = load atomic float, ptr %ptr seq_cst, align 4, !some.unknown.md !0
@@ -50,8 +46,7 @@ define float @load_atomic_f32_flat_system(ptr %ptr) {
 define float @load_atomic_f32_flat_agent(ptr %ptr) {
 ; CHECK-LABEL: define float @load_atomic_f32_flat_agent(
 ; CHECK-SAME: ptr [[PTR:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = load atomic i32, ptr [[PTR]] syncscope("agent") seq_cst, align 4
-; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i32 [[TMP1]] to float
+; CHECK-NEXT:    [[TMP2:%.*]] = load atomic float, ptr [[PTR]] syncscope("agent") seq_cst, align 4, !some.unknown.md [[META0]]
 ; CHECK-NEXT:    ret float [[TMP2]]
 ;
   %ld = load atomic float, ptr %ptr syncscope("agent") seq_cst, align 4, !some.unknown.md !0
@@ -61,8 +56,7 @@ define float @load_atomic_f32_flat_agent(ptr %ptr) {
 define half @load_atomic_f16_global_system(ptr addrspace(1) %ptr) {
 ; CHECK-LABEL: define half @load_atomic_f16_global_system(
 ; CHECK-SAME: ptr addrspace(1) [[PTR:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = load atomic i16, ptr addrspace(1) [[PTR]] seq_cst, align 4
-; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i16 [[TMP1]] to half
+; CHECK-NEXT:    [[TMP2:%.*]] = load atomic half, ptr addrspace(1) [[PTR]] seq_cst, align 4, !some.unknown.md [[META0]]
 ; CHECK-NEXT:    ret half [[TMP2]]
 ;
   %ld = load atomic half, ptr addrspace(1) %ptr seq_cst, align 4, !some.unknown.md !0
@@ -72,8 +66,7 @@ define half @load_atomic_f16_global_system(ptr addrspace(1) %ptr) {
 define half @load_atomic_f16_global_agent(ptr addrspace(1) %ptr) {
 ; CHECK-LABEL: define half @load_atomic_f16_global_agent(
 ; CHECK-SAME: ptr addrspace(1) [[PTR:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = load atomic i16, ptr addrspace(1) [[PTR]] syncscope("agent") seq_cst, align 4
-; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i16 [[TMP1]] to half
+; CHECK-NEXT:    [[TMP2:%.*]] = load atomic half, ptr addrspace(1) [[PTR]] syncscope("agent") seq_cst, align 4, !some.unknown.md [[META0]]
 ; CHECK-NEXT:    ret half [[TMP2]]
 ;
   %ld = load atomic half, ptr addrspace(1) %ptr syncscope("agent") seq_cst, align 4, !some.unknown.md !0
@@ -83,8 +76,7 @@ define half @load_atomic_f16_global_agent(ptr addrspace(1) %ptr) {
 define half @load_atomic_f16_local(ptr addrspace(3) %ptr) {
 ; CHECK-LABEL: define half @load_atomic_f16_local(
 ; CHECK-SAME: ptr addrspace(3) [[PTR:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = load atomic i16, ptr addrspace(3) [[PTR]] seq_cst, align 2
-; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i16 [[TMP1]] to half
+; CHECK-NEXT:    [[TMP2:%.*]] = load atomic half, ptr addrspace(3) [[PTR]] seq_cst, align 2, !some.unknown.md [[META0]]
 ; CHECK-NEXT:    ret half [[TMP2]]
 ;
   %ld = load atomic half, ptr addrspace(3) %ptr seq_cst, align 2, !some.unknown.md !0
@@ -94,8 +86,7 @@ define half @load_atomic_f16_local(ptr addrspace(3) %ptr) {
 define bfloat @load_atomic_bf16_global_system(ptr addrspace(1) %ptr) {
 ; CHECK-LABEL: define bfloat @load_atomic_bf16_global_system(
 ; CHECK-SAME: ptr addrspace(1) [[PTR:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = load atomic i16, ptr addrspace(1) [[PTR]] seq_cst, align 2
-; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i16 [[TMP1]] to bfloat
+; CHECK-NEXT:    [[TMP2:%.*]] = load atomic bfloat, ptr addrspace(1) [[PTR]] seq_cst, align 2, !some.unknown.md [[META0]]
 ; CHECK-NEXT:    ret bfloat [[TMP2]]
 ;
   %ld = load atomic bfloat, ptr addrspace(1) %ptr seq_cst, align 2, !some.unknown.md !0
@@ -105,8 +96,7 @@ define bfloat @load_atomic_bf16_global_system(ptr addrspace(1) %ptr) {
 define bfloat @load_atomic_bf16_global_agent(ptr addrspace(1) %ptr) {
 ; CHECK-LABEL: define bfloat @load_atomic_bf16_global_agent(
 ; CHECK-SAME: ptr addrspace(1) [[PTR:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = load atomic i16, ptr addrspace(1) [[PTR]] syncscope("agent") seq_cst, align 2
-; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i16 [[TMP1]] to bfloat
+; CHECK-NEXT:    [[TMP2:%.*]] = load atomic bfloat, ptr addrspace(1) [[PTR]] syncscope("agent") seq_cst, align 2, !some.unknown.md [[META0]]
 ; CHECK-NEXT:    ret bfloat [[TMP2]]
 ;
   %ld = load atomic bfloat, ptr addrspace(1) %ptr syncscope("agent") seq_cst, align 2, !some.unknown.md !0
@@ -116,8 +106,7 @@ define bfloat @load_atomic_bf16_global_agent(ptr addrspace(1) %ptr) {
 define bfloat @load_atomic_bf16_local(ptr addrspace(3) %ptr) {
 ; CHECK-LABEL: define bfloat @load_atomic_bf16_local(
 ; CHECK-SAME: ptr addrspace(3) [[PTR:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = load atomic i16, ptr addrspace(3) [[PTR]] seq_cst, align 2
-; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i16 [[TMP1]] to bfloat
+; CHECK-NEXT:    [[TMP2:%.*]] = load atomic bfloat, ptr addrspace(3) [[PTR]] seq_cst, align 2, !some.unknown.md [[META0]]
 ; CHECK-NEXT:    ret bfloat [[TMP2]]
 ;
   %ld = load atomic bfloat, ptr addrspace(3) %ptr seq_cst, align 2, !some.unknown.md !0
@@ -127,8 +116,7 @@ define bfloat @load_atomic_bf16_local(ptr addrspace(3) %ptr) {
 define bfloat @load_atomic_bf16_flat(ptr %ptr) {
 ; CHECK-LABEL: define bfloat @load_atomic_bf16_flat(
 ; CHECK-SAME: ptr [[PTR:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = load atomic i16, ptr [[PTR]] seq_cst, align 2
-; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i16 [[TMP1]] to bfloat
+; CHECK-NEXT:    [[TMP2:%.*]] = load atomic bfloat, ptr [[PTR]] seq_cst, align 2, !some.unknown.md [[META0]]
 ; CHECK-NEXT:    ret bfloat [[TMP2]]
 ;
   %ld = load atomic bfloat, ptr %ptr seq_cst, align 2, !some.unknown.md !0
@@ -138,8 +126,7 @@ define bfloat @load_atomic_bf16_flat(ptr %ptr) {
 define double @load_atomic_f64_global_system(ptr addrspace(1) %ptr) {
 ; CHECK-LABEL: define double @load_atomic_f64_global_system(
 ; CHECK-SAME: ptr addrspace(1) [[PTR:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = load atomic i64, ptr addrspace(1) [[PTR]] seq_cst, align 8
-; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i64 [[TMP1]] to double
+; CHECK-NEXT:    [[TMP2:%.*]] = load atomic double, ptr addrspace(1) [[PTR]] seq_cst, align 8, !some.unknown.md [[META0]]
 ; CHECK-NEXT:    ret double [[TMP2]]
 ;
   %ld = load atomic double, ptr addrspace(1) %ptr seq_cst, align 8, !some.unknown.md !0
@@ -149,8 +136,7 @@ define double @load_atomic_f64_global_system(ptr addrspace(1) %ptr) {
 define double @load_atomic_f64_global_agent(ptr addrspace(1) %ptr) {
 ; CHECK-LABEL: define double @load_atomic_f64_global_agent(
 ; CHECK-SAME: ptr addrspace(1) [[PTR:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = load atomic i64, ptr addrspace(1) [[PTR]] syncscope("agent") seq_cst, align 8
-; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i64 [[TMP1]] to double
+; CHECK-NEXT:    [[TMP2:%.*]] = load atomic double, ptr addrspace(1) [[PTR]] syncscope("agent") seq_cst, align 8, !some.unknown.md [[META0]]
 ; CHECK-NEXT:    ret double [[TMP2]]
 ;
   %ld = load atomic double, ptr addrspace(1) %ptr syncscope("agent") seq_cst, align 8, !some.unknown.md !0
@@ -160,8 +146,7 @@ define double @load_atomic_f64_global_agent(ptr addrspace(1) %ptr) {
 define double @load_atomic_f64_local(ptr addrspace(3) %ptr) {
 ; CHECK-LABEL: define double @load_atomic_f64_local(
 ; CHECK-SAME: ptr addrspace(3) [[PTR:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = load atomic i64, ptr addrspace(3) [[PTR]] seq_cst, align 8
-; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i64 [[TMP1]] to double
+; CHECK-NEXT:    [[TMP2:%.*]] = load atomic double, ptr addrspace(3) [[PTR]] seq_cst, align 8, !some.unknown.md [[META0]]
 ; CHECK-NEXT:    ret double [[TMP2]]
 ;
   %ld = load atomic double, ptr addrspace(3) %ptr seq_cst, align 8, !some.unknown.md !0
@@ -171,8 +156,7 @@ define double @load_atomic_f64_local(ptr addrspace(3) %ptr) {
 define double @load_atomic_f64_flat_system(ptr %ptr) {
 ; CHECK-LABEL: define double @load_atomic_f64_flat_system(
 ; CHECK-SAME: ptr [[PTR:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = load atomic i64, ptr [[PTR]] seq_cst, align 8
-; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i64 [[TMP1]] to double
+; CHECK-NEXT:    [[TMP2:%.*]] = load atomic double, ptr [[PTR]] seq_cst, align 8, !some.unknown.md [[META0]]
 ; CHECK-NEXT:    ret double [[TMP2]]
 ;
   %ld = load atomic double, ptr %ptr seq_cst, align 8, !some.unknown.md !0
@@ -182,8 +166,7 @@ define double @load_atomic_f64_flat_system(ptr %ptr) {
 define double @load_atomic_f64_flat_agent(ptr %ptr) {
 ; CHECK-LABEL: define double @load_atomic_f64_flat_agent(
 ; CHECK-SAME: ptr [[PTR:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = load atomic i64, ptr [[PTR]] syncscope("agent") seq_cst, align 8
-; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i64 [[TMP1]] to double
+; CHECK-NEXT:    [[TMP2:%.*]] = load atomic double, ptr [[PTR]] syncscope("agent") seq_cst, align 8, !some.unknown.md [[META0]]
 ; CHECK-NEXT:    ret double [[TMP2]]
 ;
   %ld = load atomic double, ptr %ptr syncscope("agent") seq_cst, align 8, !some.unknown.md !0
@@ -193,3 +176,6 @@ define double @load_atomic_f64_flat_agent(ptr %ptr) {
 !0 = !{}
 
 
+;.
+; CHECK: [[META0]] = !{}
+;.

Copy link
Contributor

@shiltian shiltian left a comment

Choose a reason for hiding this comment

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

LG

@@ -230,6 +230,12 @@ class AMDGPUTargetLowering : public TargetLowering {
bool isCheapToSpeculateCtlz(Type *Ty) const override;

bool isSDNodeAlwaysUniform(const SDNode *N) const override;

// FIXME: This hook should not exist
Copy link
Member

Choose a reason for hiding this comment

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

Are you planning any follow-up work here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'm most of the way through getting amdgpu to only bitcast in the DAG. Then theoretically it should be easy to remove it. The atomicrmw case is giving me a bit of trouble, since it seems there's some value in keeping bitcasts out of the cmpxchg loop

@arsenm arsenm force-pushed the amdgpu-no-bitcast-atomic-load-in-ir branch 2 times, most recently from e98e2e3 to cf2de93 Compare April 26, 2024 07:53
These hooks should be removed. This is a trivial legalization transform
the legalizer needs to support. The IR just complicates things, and
it was losing metadata. Implement the DAG promotion support, and
switch AMDGPU over to using it.

Really we'd be a lot better off merging ATOMIC_LOAD and LOAD like
GlobalISel does.
@arsenm arsenm force-pushed the amdgpu-no-bitcast-atomic-load-in-ir branch from cf2de93 to a6a3d43 Compare April 26, 2024 09:20
@arsenm
Copy link
Contributor Author

arsenm commented Apr 26, 2024

Bot keeps reporting failures with no failures in the log

@arsenm arsenm merged commit f1112eb into llvm:main Apr 26, 2024
3 of 4 checks passed
@arsenm arsenm deleted the amdgpu-no-bitcast-atomic-load-in-ir branch April 26, 2024 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants