Skip to content

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 11, 2025
Merged

AMDGPU: Remove "gws" feature from generic targets #148122

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions llvm/lib/Target/AMDGPU/GCNProcessors.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Copy link
Collaborator

@rampitec rampitec Jul 11, 2025

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.

Copy link
Contributor

@arsenm arsenm Jul 11, 2025

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

Copy link
Contributor Author

@changpeng changpeng Jul 11, 2025 •
edited
Loading

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.

Copy link
Collaborator

@rampitec rampitec Jul 11, 2025

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.

Copy link
Contributor

@arsenm arsenm Jul 11, 2025

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

Copy link
Contributor Author

@changpeng changpeng Jul 11, 2025

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.

Copy link
Collaborator

@rampitec rampitec Jul 11, 2025 •
edited
Loading

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.

Copy link
Contributor

@arsenm arsenm Jul 11, 2025

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

>;

def : ProcessorModel<"generic-hsa", NoSchedModel,
[FeatureGDS, FeatureGWS, FeatureFlatAddressSpace]
[FeatureGDS, FeatureFlatAddressSpace]
>;

//===------------------------------------------------------------===//
Expand Down
Loading