-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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] Remove Base_MUBUF_Real_Atomic_gfx11. NFC. #83994
Conversation
This class only existed to set the dlc bit for GFX11 atomics. It is simpler to set dlc for all loads/stores/atomics in the base class.
@llvm/pr-subscribers-backend-amdgpu Author: Jay Foad (jayfoad) ChangesThis class only existed to set the dlc bit for GFX11 atomics. It is Full diff: https://github.com/llvm/llvm-project/pull/83994.diff 1 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/BUFInstructions.td b/llvm/lib/Target/AMDGPU/BUFInstructions.td
index a2ff56d68218cd..20e91da4995b7b 100644
--- a/llvm/lib/Target/AMDGPU/BUFInstructions.td
+++ b/llvm/lib/Target/AMDGPU/BUFInstructions.td
@@ -2301,7 +2301,8 @@ class MUBUF_Real_gfx11<bits<8> op, MUBUF_Pseudo ps,
string real_name = ps.Mnemonic> :
Base_MUBUF_Real_gfx6_gfx7_gfx10_gfx11<ps, SIEncodingFamily.GFX11, real_name> {
let Inst{12} = !if(ps.has_slc, cpol{CPolBit.SLC}, ?);
- let Inst{13} = !if(ps.has_dlc, cpol{CPolBit.DLC}, ps.dlc_value);
+ // In GFX11 dlc is applicable to all loads/stores/atomics.
+ let Inst{13} = !if(!or(ps.mayLoad, ps.mayStore), cpol{CPolBit.DLC}, ps.dlc_value);
let Inst{14} = !if(ps.has_glc, cpol{CPolBit.GLC}, ps.glc_value);
let Inst{25-18} = op;
let Inst{53} = ps.tfe;
@@ -2311,12 +2312,6 @@ class MUBUF_Real_gfx11<bits<8> op, MUBUF_Pseudo ps,
let DecoderNamespace = "GFX11";
}
-class Base_MUBUF_Real_Atomic_gfx11<bits<8> op, MUBUF_Pseudo ps,
- string real_name> :
- MUBUF_Real_gfx11<op, ps, real_name> {
- let Inst{13} = cpol{CPolBit.DLC};
-}
-
class Base_MUBUF_Real_gfx6_gfx7_gfx10<bits<7> op, MUBUF_Pseudo ps, int ef> :
Base_MUBUF_Real_gfx6_gfx7_gfx10_gfx11<ps, ef> {
let Inst{12} = ps.offen;
@@ -2499,7 +2494,7 @@ multiclass MUBUF_Real_AllAddr_gfx11_gfx12_Renamed<bits<8> op, string real_name>
class MUBUF_Real_Atomic_gfx11_impl<bits<8> op, string ps_name,
string real_name> :
- Base_MUBUF_Real_Atomic_gfx11<op, !cast<MUBUF_Pseudo>(ps_name), real_name>;
+ MUBUF_Real_gfx11<op, !cast<MUBUF_Pseudo>(ps_name), real_name>;
class MUBUF_Real_Atomic_gfx12_impl<bits<8> op, string ps_name,
string real_name> :
|
@@ -2301,7 +2301,8 @@ class MUBUF_Real_gfx11<bits<8> op, MUBUF_Pseudo ps, | |||
string real_name = ps.Mnemonic> : | |||
Base_MUBUF_Real_gfx6_gfx7_gfx10_gfx11<ps, SIEncodingFamily.GFX11, real_name> { | |||
let Inst{12} = !if(ps.has_slc, cpol{CPolBit.SLC}, ?); | |||
let Inst{13} = !if(ps.has_dlc, cpol{CPolBit.DLC}, ps.dlc_value); | |||
// In GFX11 dlc is applicable to all loads/stores/atomics. | |||
let Inst{13} = !if(!or(ps.mayLoad, ps.mayStore), cpol{CPolBit.DLC}, ps.dlc_value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could perhaps use some further cleanup. The confusion is that has_dlc is a field in the pseudo, but in fact whether an instruction "has dlc" is different on different subtargets:
- before GFX11 only loads and stores have dlc
- in GFX11, atomics also have dlc (but invalidates still do not)
@@ -2499,7 +2494,7 @@ multiclass MUBUF_Real_AllAddr_gfx11_gfx12_Renamed<bits<8> op, string real_name> | |||
|
|||
class MUBUF_Real_Atomic_gfx11_impl<bits<8> op, string ps_name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the next thing to get rid of?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I am working on a larger patch to remove some useless *_impl
classes.
This class only existed to set the dlc bit for GFX11 atomics. It is
simpler to set dlc for all loads/stores/atomics in the base class.