[AMDGPU] Flip the default value of maybeAtomic. NFCI. #75220
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In practice maybeAtomic = 0 is used to prevent SIMemoryLegalizer from interfering with instructions that are mayLoad or mayStore but lack MachineMemOperands. These instructions should be the exception not the rule, so this patch sets maybeAtomic = 1 by default and only overrides it to 0 where necessary.
|
@llvm/pr-subscribers-backend-amdgpu Author: Jay Foad (jayfoad) ChangesIn practice maybeAtomic = 0 is used to prevent SIMemoryLegalizer from Full diff: https://github.com/llvm/llvm-project/pull/75220.diff 8 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/BUFInstructions.td b/llvm/lib/Target/AMDGPU/BUFInstructions.td
index 44fd4ef864127..4696ea47f9cef 100644
--- a/llvm/lib/Target/AMDGPU/BUFInstructions.td
+++ b/llvm/lib/Target/AMDGPU/BUFInstructions.td
@@ -477,7 +477,6 @@ class MUBUF_Load_Pseudo <string opName,
let has_vdata = !not(!or(isLds, isLdsOpc));
let mayLoad = 1;
let mayStore = isLds;
- let maybeAtomic = 1;
let Uses = !if(!or(isLds, isLdsOpc) , [EXEC, M0], [EXEC]);
let tfe = isTFE;
let lds = isLds;
@@ -567,7 +566,6 @@ class MUBUF_Store_Pseudo <string opName,
getAddrName<addrKindCopy>.ret;
let mayLoad = 0;
let mayStore = 1;
- let maybeAtomic = 1;
let elements = getMUBUFElements<store_vt>.ret;
let tfe = isTFE;
}
@@ -618,7 +616,6 @@ class MUBUF_Pseudo_Store_Lds<string opName>
let LGKM_CNT = 1;
let mayLoad = 1;
let mayStore = 1;
- let maybeAtomic = 1;
let has_vdata = 0;
let has_vaddr = 0;
@@ -680,7 +677,6 @@ class MUBUF_Atomic_Pseudo<string opName,
let has_glc = 0;
let has_dlc = 0;
let has_sccb = 1;
- let maybeAtomic = 1;
let AsmMatchConverter = "cvtMubufAtomic";
}
diff --git a/llvm/lib/Target/AMDGPU/DSInstructions.td b/llvm/lib/Target/AMDGPU/DSInstructions.td
index 1a10a8fcaadca..b713e94a01caa 100644
--- a/llvm/lib/Target/AMDGPU/DSInstructions.td
+++ b/llvm/lib/Target/AMDGPU/DSInstructions.td
@@ -19,7 +19,6 @@ class DS_Pseudo <string opName, dag outs, dag ins, string asmOps, list<dag> patt
// Most instruction load and store data, so set this as the default.
let mayLoad = 1;
let mayStore = 1;
- let maybeAtomic = 1;
let hasSideEffects = 0;
let SchedRW = [WriteLDS];
diff --git a/llvm/lib/Target/AMDGPU/EXPInstructions.td b/llvm/lib/Target/AMDGPU/EXPInstructions.td
index ff1d661ef6fe1..4cfee7d013ef1 100644
--- a/llvm/lib/Target/AMDGPU/EXPInstructions.td
+++ b/llvm/lib/Target/AMDGPU/EXPInstructions.td
@@ -20,6 +20,7 @@ class EXPCommon<bit row, bit done, string asm = ""> : InstSI<
let EXP_CNT = 1;
let mayLoad = done;
let mayStore = 1;
+ let maybeAtomic = 0;
let UseNamedOperandTable = 1;
let Uses = !if(row, [EXEC, M0], [EXEC]);
let SchedRW = [WriteExport];
diff --git a/llvm/lib/Target/AMDGPU/FLATInstructions.td b/llvm/lib/Target/AMDGPU/FLATInstructions.td
index c0251164faee8..a1ff3af663352 100644
--- a/llvm/lib/Target/AMDGPU/FLATInstructions.td
+++ b/llvm/lib/Target/AMDGPU/FLATInstructions.td
@@ -173,7 +173,6 @@ class FLAT_Load_Pseudo <string opName, RegisterClass regClass,
let has_saddr = HasSaddr;
let enabled_saddr = EnableSaddr;
let PseudoInstr = opName#!if(!and(HasSaddr, EnableSaddr), "_SADDR", "");
- let maybeAtomic = 1;
let Constraints = !if(HasTiedOutput, "$vdst = $vdst_in", "");
let DisableEncoding = !if(HasTiedOutput, "$vdst_in", "");
@@ -195,7 +194,6 @@ class FLAT_Store_Pseudo <string opName, RegisterClass vdataClass,
let has_saddr = HasSaddr;
let enabled_saddr = EnableSaddr;
let PseudoInstr = opName#!if(!and(HasSaddr, EnableSaddr), "_SADDR", "");
- let maybeAtomic = 1;
}
multiclass FLAT_Global_Load_Pseudo<string opName, RegisterClass regClass, bit HasTiedInput = 0> {
@@ -221,7 +219,6 @@ class FLAT_Global_Load_AddTid_Pseudo <string opName, RegisterClass regClass,
let has_vaddr = 0;
let has_saddr = 1;
let enabled_saddr = EnableSaddr;
- let maybeAtomic = 1;
let PseudoInstr = opName#!if(EnableSaddr, "_SADDR", "");
let Constraints = !if(HasTiedOutput, "$vdst = $vdst_in", "");
@@ -288,7 +285,6 @@ class FLAT_Global_Store_AddTid_Pseudo <string opName, RegisterClass vdataClass,
let has_vaddr = 0;
let has_saddr = 1;
let enabled_saddr = EnableSaddr;
- let maybeAtomic = 1;
let PseudoInstr = opName#!if(EnableSaddr, "_SADDR", "");
}
@@ -331,7 +327,6 @@ class FLAT_Scratch_Load_Pseudo <string opName, RegisterClass regClass,
let has_sve = EnableSVE;
let sve = EnableVaddr;
let PseudoInstr = opName#!if(EnableSVE, "_SVS", !if(EnableSaddr, "_SADDR", !if(EnableVaddr, "", "_ST")));
- let maybeAtomic = 1;
let Constraints = !if(HasTiedOutput, "$vdst = $vdst_in", "");
let DisableEncoding = !if(HasTiedOutput, "$vdst_in", "");
@@ -360,7 +355,6 @@ class FLAT_Scratch_Store_Pseudo <string opName, RegisterClass vdataClass, bit En
let has_sve = EnableSVE;
let sve = EnableVaddr;
let PseudoInstr = opName#!if(EnableSVE, "_SVS", !if(EnableSaddr, "_SADDR", !if(EnableVaddr, "", "_ST")));
- let maybeAtomic = 1;
}
multiclass FLAT_Scratch_Load_Pseudo<string opName, RegisterClass regClass, bit HasTiedOutput = 0> {
@@ -450,7 +444,6 @@ class FLAT_AtomicNoRet_Pseudo<string opName, dag outs, dag ins,
let has_vdst = 0;
let has_sccb = 1;
let sccbValue = 0;
- let maybeAtomic = 1;
let IsAtomicNoRet = 1;
}
diff --git a/llvm/lib/Target/AMDGPU/LDSDIRInstructions.td b/llvm/lib/Target/AMDGPU/LDSDIRInstructions.td
index 4956a15867749..bcad5ff5ecea7 100644
--- a/llvm/lib/Target/AMDGPU/LDSDIRInstructions.td
+++ b/llvm/lib/Target/AMDGPU/LDSDIRInstructions.td
@@ -48,6 +48,7 @@ class LDSDIR_Common<string opName, string asm = "", bit direct> : InstSI<
let hasSideEffects = 0;
let mayLoad = 1;
let mayStore = 0;
+ let maybeAtomic = 0;
string Mnemonic = opName;
let UseNamedOperandTable = 1;
diff --git a/llvm/lib/Target/AMDGPU/SIInstrFormats.td b/llvm/lib/Target/AMDGPU/SIInstrFormats.td
index 585a3eb786187..1b66d163714fb 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrFormats.td
+++ b/llvm/lib/Target/AMDGPU/SIInstrFormats.td
@@ -91,7 +91,7 @@ class InstSI <dag outs, dag ins, string asm = "",
field bit VOP3_OPSEL = 0;
// Is it possible for this instruction to be atomic?
- field bit maybeAtomic = 0;
+ field bit maybeAtomic = 1;
// This bit indicates that this is a VI instruction which is renamed
// in GFX9. Required for correct mapping from pseudo to MC.
diff --git a/llvm/lib/Target/AMDGPU/SIInstructions.td b/llvm/lib/Target/AMDGPU/SIInstructions.td
index 9362fe5d9678b..47b4e1a5c148f 100644
--- a/llvm/lib/Target/AMDGPU/SIInstructions.td
+++ b/llvm/lib/Target/AMDGPU/SIInstructions.td
@@ -111,7 +111,6 @@ def ATOMIC_FENCE : SPseudoInstSI<
[(atomic_fence (i32 timm:$ordering), (i32 timm:$scope))],
"ATOMIC_FENCE $ordering, $scope"> {
let hasSideEffects = 1;
- let maybeAtomic = 1;
}
let hasSideEffects = 0, mayLoad = 0, mayStore = 0, Uses = [EXEC] in {
@@ -557,6 +556,7 @@ def SI_MASKED_UNREACHABLE : SPseudoInstSI <(outs), (ins),
let hasNoSchedulingInfo = 1;
let FixedSize = 1;
let isMeta = 1;
+ let maybeAtomic = 0;
}
// Used as an isel pseudo to directly emit initialization with an
diff --git a/llvm/lib/Target/AMDGPU/SMInstructions.td b/llvm/lib/Target/AMDGPU/SMInstructions.td
index c18846483cf95..323f49ab91f01 100644
--- a/llvm/lib/Target/AMDGPU/SMInstructions.td
+++ b/llvm/lib/Target/AMDGPU/SMInstructions.td
@@ -29,6 +29,7 @@ class SM_Pseudo <string opName, dag outs, dag ins, string asmOps, list<dag> patt
let mayStore = 0;
let mayLoad = 1;
let hasSideEffects = 0;
+ let maybeAtomic = 0;
let UseNamedOperandTable = 1;
let SchedRW = [WriteSMEM];
|
jayfoad
commented
Dec 12, 2023
| @@ -29,6 +29,7 @@ class SM_Pseudo <string opName, dag outs, dag ins, string asmOps, list<dag> patt | |||
| let mayStore = 0; | |||
| let mayLoad = 1; | |||
| let hasSideEffects = 0; | |||
| let maybeAtomic = 0; | |||
There was a problem hiding this comment.
This preserves the existing behaviour but it feels wrong. In practice it means that SIMemoryLegalizer does not set glc on volatile SMEM loads.
There was a problem hiding this comment.
Probably should fix this in a separate change
arsenm
approved these changes
Jan 9, 2024
| @@ -29,6 +29,7 @@ class SM_Pseudo <string opName, dag outs, dag ins, string asmOps, list<dag> patt | |||
| let mayStore = 0; | |||
| let mayLoad = 1; | |||
| let hasSideEffects = 0; | |||
| let maybeAtomic = 0; | |||
There was a problem hiding this comment.
Probably should fix this in a separate change
justinfargnoli
pushed a commit
to justinfargnoli/llvm-project
that referenced
this pull request
Jan 28, 2024
In practice maybeAtomic = 0 is used to prevent SIMemoryLegalizer from interfering with instructions that are mayLoad or mayStore but lack MachineMemOperands. These instructions should be the exception not the rule, so this patch sets maybeAtomic = 1 by default and only overrides it to 0 where necessary.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In practice maybeAtomic = 0 is used to prevent SIMemoryLegalizer from
interfering with instructions that are mayLoad or mayStore but lack
MachineMemOperands. These instructions should be the exception not the
rule, so this patch sets maybeAtomic = 1 by default and only overrides
it to 0 where necessary.