Skip to content

Conversation

@changpeng
Copy link
Contributor

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

  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
@llvmbot
Copy link
Member

llvmbot commented Jul 11, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Changpeng Fang (changpeng)

Changes

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


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

1 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/GCNProcessors.td (+2-2)
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]
 >;
 
 //===------------------------------------------------------------===//

@changpeng changpeng requested review from rampitec and shiltian July 11, 2025 07:03
@changpeng changpeng merged commit edd615e into llvm:main Jul 11, 2025
9 of 11 checks passed
@changpeng changpeng deleted the gws branch July 11, 2025 07:07
Copy link
Collaborator

@rampitec rampitec left a 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]
Copy link
Collaborator

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

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

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

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

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

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

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

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

rahulc-gh pushed a commit to ROCm/llvm-project that referenced this pull request Aug 7, 2025
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants