[SelectionDAG] Legalize <1 x T> vector types for atomic load #111414

jofrn · 2024-10-07T17:57:45Z

load atomic <1 x T> is not valid. This change legalizes vector types of atomic load via scalarization in SelectionDAG so that it can, for example, translate from v1i32 to i32.

llvmbot · 2024-10-07T17:58:20Z

@llvm/pr-subscribers-llvm-ir
@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-llvm-selectiondag

Author: None (jofrn)

Changes

Scalarize vector of atomic load in SelectionDAG.

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

3 Files Affected:

(modified) llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h (+1)
(modified) llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp (+16)
(added) llvm/test/CodeGen/Generic/atomic-scalarization.ll (+17)

diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
index d14516ef3e2fbb..5204f20e97f63e 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
@@ -850,6 +850,7 @@ class LLVM_LIBRARY_VISIBILITY DAGTypeLegalizer {
   SDValue ScalarizeVecRes_ExpOp(SDNode *N);
   SDValue ScalarizeVecRes_INSERT_VECTOR_ELT(SDNode *N);
   SDValue ScalarizeVecRes_LOAD(LoadSDNode *N);
+  SDValue ScalarizeVecRes_ATOMIC_LOAD(AtomicSDNode *N);
   SDValue ScalarizeVecRes_SCALAR_TO_VECTOR(SDNode *N);
   SDValue ScalarizeVecRes_VSELECT(SDNode *N);
   SDValue ScalarizeVecRes_SELECT(SDNode *N);
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
index 0a22f06271984e..1e911da130f8c3 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
@@ -60,6 +60,9 @@ void DAGTypeLegalizer::ScalarizeVectorResult(SDNode *N, unsigned ResNo) {
   case ISD::FP_ROUND:          R = ScalarizeVecRes_FP_ROUND(N); break;
   case ISD::FPOWI:             R = ScalarizeVecRes_ExpOp(N); break;
   case ISD::INSERT_VECTOR_ELT: R = ScalarizeVecRes_INSERT_VECTOR_ELT(N); break;
+  case ISD::ATOMIC_LOAD:
+    R = ScalarizeVecRes_ATOMIC_LOAD(cast<AtomicSDNode>(N));
+    break;
   case ISD::LOAD:           R = ScalarizeVecRes_LOAD(cast<LoadSDNode>(N));break;
   case ISD::SCALAR_TO_VECTOR:  R = ScalarizeVecRes_SCALAR_TO_VECTOR(N); break;
   case ISD::SIGN_EXTEND_INREG: R = ScalarizeVecRes_InregOp(N); break;
@@ -447,6 +450,19 @@ SDValue DAGTypeLegalizer::ScalarizeVecRes_INSERT_VECTOR_ELT(SDNode *N) {
   return Op;
 }
 
+SDValue DAGTypeLegalizer::ScalarizeVecRes_ATOMIC_LOAD(AtomicSDNode *N) {
+
+  SDValue Result = DAG.getAtomic(ISD::ATOMIC_LOAD, SDLoc(N),
+      N->getMemoryVT().getVectorElementType(),
+      N->getValueType(0).getVectorElementType(),
+      N->getChain(), N->getBasePtr(), N->getMemOperand());
+
+  // Legalize the chain result - switch anything that used the old chain to
+  // use the new one.
+  ReplaceValueWith(SDValue(N, 1), Result.getValue(1));
+  return Result;
+}
+
 SDValue DAGTypeLegalizer::ScalarizeVecRes_LOAD(LoadSDNode *N) {
   assert(N->isUnindexed() && "Indexed vector load?");
 
diff --git a/llvm/test/CodeGen/Generic/atomic-scalarization.ll b/llvm/test/CodeGen/Generic/atomic-scalarization.ll
new file mode 100644
index 00000000000000..3f611323172884
--- /dev/null
+++ b/llvm/test/CodeGen/Generic/atomic-scalarization.ll
@@ -0,0 +1,17 @@
+; RUN: llc %s --print-after-isel --disable-verify 2>&1 | FileCheck %s
+
+define i32 @atomic_scalar() {
+; CHECK: # After Instruction Selection:
+; CHECK-NEXT: # Machine code for function atomic_scalar: IsSSA, TracksLiveness
+; CHECK-NEXT: Frame Objects:
+; CHECK-NEXT:   fi#0: size=4, align=4, at location [SP+8]
+; CHECK:      bb.0 (%ir-block.0):
+; CHECK-NEXT:   %0:gr32 = MOV32rm %stack.0, 1, $noreg, 0, $noreg :: (dereferenceable load acquire (s32) from %ir.1)
+; CHECK-NEXT:   $eax = COPY %0:gr32
+; CHECK-NEXT:   RET 0, $eax
+; CHECK:      # End machine code for function atomic_scalar.
+  %1 = alloca <1 x i32>
+  %2 = load atomic <1 x i32>, ptr %1 acquire, align 4
+  %3 = extractelement <1 x i32> %2, i32 0
+  ret i32 %3
+}

github-actions · 2024-10-07T18:01:41Z

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

llvm/test/CodeGen/Generic/atomic-scalarization.ll

llvm/test/CodeGen/X86/atomic-scalarization.ll

arsenm

Do stores also need to be handled?

arsenm

No disable-verify

Vector types on atomics are assumed to be invalid by the verifier. However, this type can be valid if it is lowered by codegen.

Scalarize vector of atomic load in SelectionDAG.

jofrn · 2024-12-05T10:32:39Z

| Do stores also need to be handled?
No, they do not.
| No disable-verify
Rebased this on top of #117625, which permits vector types of atomics in IR/Verifier.

arsenm · 2024-12-05T18:28:35Z

| Do stores also need to be handled? No, they do not.

They do, but it can be a separate pr

arsenm

codegen parts lgtm, but should avoid manually stacking PRs (which really doesn't work). It's much harder to review, if you want to stack PRs use graphite or spr

llvm/test/CodeGen/X86/atomic-scalarization.ll

jofrn · 2024-12-06T22:07:06Z

llvm/test/CodeGen/X86/atomic-load-store.ll

@@ -1,12 +1,18 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=x86_64-apple-macosx10.7.0 -verify-machineinstrs | FileCheck %s
 ; RUN: llc < %s -mtriple=x86_64-apple-macosx10.7.0 -verify-machineinstrs -O0 | FileCheck %s
+; RUN: llc %s --mtriple=x86_64 -o - | FileCheck %s --check-prefix=CHECK0



This seems like a good preexisting test to add the <1 x n> cases to. It has a specific triple apple, and when generalizing it, the difference in the output tests is not much (a #), and so it duplicates quite a bit between x86_64 and apple. However, I do notice that bfloat passes only when --mtriple=x86_64, so we may want to add the additional run line.

Just copy the mtriple that's already here, the OS should not be significant for the purpose here. I'm surprised this also doesn't have a 32-bit run line

arsenm

This patch will only cover 1 x vectors. The patch is fine as-is, but the message should change to reflect this. An additional step is required to handle any other vector size

arsenm · 2024-12-12T11:40:42Z

This patch will only cover 1 x vectors. The patch is fine as-is, but the message should change to reflect this. An additional step is required to handle any other vector size

Still only handles 1 x vectors

llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp

RKSimon · 2024-12-17T22:42:22Z

llvm/test/CodeGen/X86/atomic-load-store.ll

+; CHECK-NEXT:    retq
+  %ret = load atomic <1 x i32>, ptr %x acquire, align 4
+  ret <1 x i32> %ret
+}


add atomic load extension test coverage?

RKSimon · 2024-12-17T22:43:01Z

llvm/test/Verifier/atomics.ll

-  ret <1 x i64> %v
+define %ty @bar(ptr %P) {
+  %v = load atomic %ty, ptr %P unordered, align 8
+  ret %ty %v
 }


please can you add <1 x i64> load/store test coverage some place

llvm-project/llvm/test/CodeGen/X86/atomic-load-store.ll

Line 113 in fb8e577

define <1 x i64> @atomic_vec1_i64(ptr %x) {

Just added it here https://github.com/llvm/llvm-project/pull/120316/commits.

Also pointers (including the 32-bit addrspace on x86-64 case which is some high number)

arsenm

DAG change LGTM on top of the additional tests in the other PR

jofrn requested review from arsenm, shiltian and Pierre-vh October 7, 2024 17:57

llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label Oct 7, 2024

jofrn force-pushed the atomic-scalarization branch from 0c84583 to c154ab8 Compare October 7, 2024 18:04

arsenm reviewed Oct 7, 2024

View reviewed changes

llvm/test/CodeGen/Generic/atomic-scalarization.ll Outdated Show resolved Hide resolved

llvm/test/CodeGen/Generic/atomic-scalarization.ll Outdated Show resolved Hide resolved

llvmbot added the backend:X86 label Oct 7, 2024

arsenm reviewed Oct 7, 2024

View reviewed changes

llvm/test/CodeGen/X86/atomic-scalarization.ll Outdated Show resolved Hide resolved

arsenm approved these changes Oct 7, 2024

View reviewed changes

arsenm requested changes Oct 7, 2024

View reviewed changes

jofrn marked this pull request as draft October 7, 2024 19:07

jofrn mentioned this pull request Oct 15, 2024

[SROA] Prevent load atomic vector from being generated #112432

Open

jofrn and others added 4 commits December 1, 2024 14:37

[Verifier] Allow vector type in atomic load and store

9b0a33b

Vector types on atomics are assumed to be invalid by the verifier. However, this type can be valid if it is lowered by codegen.

Update Assembler/atomic test

aba2a03

[SelectionDAG] Legalize vector types for atomic load

10b57a1

Scalarize vector of atomic load in SelectionDAG.

Moved test and checking mir after last X86 pass

a95ff19

jofrn force-pushed the atomic-scalarization branch 2 times, most recently from edf1c95 to 9ceb765 Compare December 5, 2024 10:20

jofrn marked this pull request as ready for review December 5, 2024 10:25

llvmbot added the llvm:ir label Dec 5, 2024

jofrn requested a review from arsenm December 5, 2024 10:26

jofrn changed the title ~~[SelectionDAG] Legalize vector types for atomic load~~ [SelectionDAG][X86] Legalize vector types for atomic load Dec 5, 2024

jofrn force-pushed the atomic-scalarization branch from 9ceb765 to 1de03a8 Compare December 5, 2024 11:58

jofrn changed the title ~~[SelectionDAG][X86] Legalize vector types for atomic load~~ [SelectionDAG] Legalize vector types for atomic load Dec 5, 2024

This was referenced Dec 5, 2024

IR: Allow vector type in atomic load and store #117625

Closed

[X86] Manage atomic load of fp -> int promotion in DAG #118793

Open

jofrn force-pushed the atomic-scalarization branch from 1de03a8 to 1fb4f56 Compare December 5, 2024 12:59

Autogenerate test.

d9d4a1c

jofrn force-pushed the atomic-scalarization branch from 1fb4f56 to d9d4a1c Compare December 5, 2024 18:20

arsenm reviewed Dec 6, 2024

View reviewed changes

llvm/test/CodeGen/X86/atomic-scalarization.ll Outdated Show resolved Hide resolved

Renamed to atomic-vector

78ec95b

jofrn force-pushed the atomic-scalarization branch from 8ebf2f9 to 78ec95b Compare December 6, 2024 21:59

Add to preexisting test

009b140

jofrn commented Dec 6, 2024

View reviewed changes

arsenm requested a review from RKSimon December 8, 2024 15:33

arsenm reviewed Dec 8, 2024

View reviewed changes

Rename tests

f99ae71

arsenm reviewed Dec 12, 2024

View reviewed changes

llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp Outdated Show resolved Hide resolved

jofrn mentioned this pull request Dec 17, 2024

[X86] Add atomic vector tests for >1 sizes. #120316

Open

Fix whitespace

b738e03

RKSimon changed the title ~~[SelectionDAG] Legalize vector types for atomic load~~ [SelectionDAG] Legalize <1 x T> vector types for atomic load Dec 17, 2024

RKSimon reviewed Dec 17, 2024

View reviewed changes

arsenm approved these changes Dec 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SelectionDAG] Legalize <1 x T> vector types for atomic load #111414

[SelectionDAG] Legalize <1 x T> vector types for atomic load #111414

jofrn commented Oct 7, 2024 •

edited

Loading

llvmbot commented Oct 7, 2024 •

edited

Loading

github-actions bot commented Oct 7, 2024 •

edited

Loading

arsenm left a comment

arsenm left a comment

jofrn commented Dec 5, 2024 •

edited

Loading

arsenm commented Dec 5, 2024

arsenm left a comment

jofrn Dec 6, 2024 •

edited

Loading

arsenm Dec 8, 2024 •

edited

Loading

arsenm left a comment

arsenm commented Dec 12, 2024

RKSimon Dec 17, 2024

RKSimon Dec 17, 2024

jofrn Dec 17, 2024

arsenm Dec 18, 2024

arsenm left a comment

[SelectionDAG] Legalize <1 x T> vector types for atomic load #111414

Are you sure you want to change the base?

[SelectionDAG] Legalize <1 x T> vector types for atomic load #111414

Conversation

jofrn commented Oct 7, 2024 • edited Loading

llvmbot commented Oct 7, 2024 • edited Loading

github-actions bot commented Oct 7, 2024 • edited Loading

arsenm left a comment

Choose a reason for hiding this comment

arsenm left a comment

Choose a reason for hiding this comment

jofrn commented Dec 5, 2024 • edited Loading

arsenm commented Dec 5, 2024

arsenm left a comment

Choose a reason for hiding this comment

jofrn Dec 6, 2024 • edited Loading

Choose a reason for hiding this comment

arsenm Dec 8, 2024 • edited Loading

Choose a reason for hiding this comment

arsenm left a comment

Choose a reason for hiding this comment

arsenm commented Dec 12, 2024

RKSimon Dec 17, 2024

Choose a reason for hiding this comment

RKSimon Dec 17, 2024

Choose a reason for hiding this comment

jofrn Dec 17, 2024

Choose a reason for hiding this comment

arsenm Dec 18, 2024

Choose a reason for hiding this comment

arsenm left a comment

Choose a reason for hiding this comment

jofrn commented Oct 7, 2024 •

edited

Loading

llvmbot commented Oct 7, 2024 •

edited

Loading

github-actions bot commented Oct 7, 2024 •

edited

Loading

jofrn commented Dec 5, 2024 •

edited

Loading

jofrn Dec 6, 2024 •

edited

Loading

arsenm Dec 8, 2024 •

edited

Loading