-
Notifications
You must be signed in to change notification settings - Fork 15.1k
AMDGPU: Remove "gws" feature from generic targets #148122
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
Conversation
Here "generic targets" means when no target is specified. This is bexause gfx12 does not have this festure, and thus it is no longer universally available. Fixes: SWDEV-541399
|
@llvm/pr-subscribers-backend-amdgpu Author: Changpeng Fang (changpeng) ChangesHere "generic targets" means when no target is specified. This is because gfx12+ does not support this feature, and thus it is no longer universally available. Fixes: SWDEV-541399 Full diff: https://github.com/llvm/llvm-project/pull/148122.diff 1 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/GCNProcessors.td b/llvm/lib/Target/AMDGPU/GCNProcessors.td
index b5ffa64c3a4b4..217942c7e539c 100644
--- a/llvm/lib/Target/AMDGPU/GCNProcessors.td
+++ b/llvm/lib/Target/AMDGPU/GCNProcessors.td
@@ -9,11 +9,11 @@
// The code produced for "generic" is only useful for tests and cannot
// reasonably be expected to execute on any particular target.
def : ProcessorModel<"generic", NoSchedModel,
- [FeatureGDS, FeatureGWS]
+ [FeatureGDS]
>;
def : ProcessorModel<"generic-hsa", NoSchedModel,
- [FeatureGDS, FeatureGWS, FeatureFlatAddressSpace]
+ [FeatureGDS, FeatureFlatAddressSpace]
>;
//===------------------------------------------------------------===//
|
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.
LGTM
| // reasonably be expected to execute on any particular target. | ||
| def : ProcessorModel<"generic", NoSchedModel, | ||
| [FeatureGDS, FeatureGWS] | ||
| [FeatureGDS] |
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.
As a side note I'd remove one last feature. IMO generic shall be featureless.
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.
GDS was also removed, so it should also not be here. Not sure how it ended up here in the first place
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.
GDS was also removed, so it should also not be here. Not sure how it ended up here in the first place
I am going to remove it in a following patch. Additional work is to remove gds related LIT tests for amdgcn/amdhsa with no target.
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.
I'd even assert somewhere that feature mask for generic is zero.
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.
No tests should be removed. The tests should be moved to use an explicit target
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.
As a side note I'd remove one last feature. IMO generic shall be featureless.
Maybe we can keep FeatureFlatAddressSpace for generic-hsa to differentiate from generic.
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.
Tahiti is not the one to support it. No luck here. Technically it has it, but practically it does not. Unless we say it is not a hsa target, which it is not really.
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.
You can ignore FeatureFlatAddressSpace, it is irrelevant now. Just leave it alone
Here "generic targets" means when no target is specified. This is because gfx12+ does not support this feature, and thus it is no longer universally available. Fixes: SWDEV-541399 (cherry picked from commit edd615e)
Here "generic targets" means when no target is specified. This is because gfx12+ does not support this feature, and thus it is no longer universally available.
Fixes: SWDEV-541399