[AMDGPU] Update subtarget features for new target ID support

[kerbowa]

Summary

[AMDGPU] Update subtarget features for new target ID support

Support for XNACK and SRAMECC is not static on some GPUs. We must be able
to differentiate between different scenarios for these dynamic subtarget
features.

The possible settings are:

• Unsupported: The GPU has no support for XNACK/SRAMECC.
• Any: Preference is unspecified. Use conservative settings that can run anywhere.
• Off: Request support for XNACK/SRAMECC Off
• On: Request support for XNACK/SRAMECC On

GCNSubtarget will track the four options based on the following criteria. If
the subtarget does not support XNACK/SRAMECC we say the setting is
"Unsupported". If no subtarget features for XNACK/SRAMECC are requested we
must support "Any" mode. If the subtarget features XNACK/SRAMECC exist in the
feature string when initializing the subtarget, the settings are "On/Off".

The defaults are updated to be conservatively correct, meaning if no setting
for XNACK or SRAMECC is explicitly requested, defaults will be used which
generate code that can be run anywhere. This corresponds to the "Any" setting.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

889–892

I was hoping this eliminated the string checks when I saw this was removed previously, but this is just moving the problem. This isn't really correct since the feature parsing doesn't care about any particular instance of the feature. e.g. +xnack,-xnack is valid and the last one wins. We really shouldn't have to guess at the intent of the feature string, and only rely on the properly parsed result

902–916

I don't think these should be fatal errors. SubtargetFeature is really missing ImpliesNot

935

I'm not sure we want debug printing in the subtarget constructor

936–950

StringSwitch

954–967

dbgs() << StringSwitch

968

Single quotes

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h

65

The backend doesn't really know if the target feature was specified or why, so I don't see why we need to track the "default" concept. The default is just on if it's supported by the hardware like before

127

No reason to use virtual for this

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

500

Parsing shouldn't be necessary

504

Having to explicitly list all the targets that support this here is worse than having a subtarget feature indicating hardware support added to the processor definition

529

Parsing shouldn't be necessary

[t-tye]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h

65

The backend does need to know as it may produce different code for the 3 settings. Currently that is not being done, but for sram-ecc there would be benefit in supporting different code gen for default and on so we can use D16 instructions and exploit that they zero the other bits when in sram-ecc on mode.

Also the ELF header encodes a distinction between default and non-default. This allows the loader to enforce compatibility between multiple loaded code objects with different settings.

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

504

If subtarget features are intended to be simply an efficient way to represent the contents of the function attribute for target features then including if a target feature is supported does not seem to belong there. That is not a function attribute that makes sense for the user to specify as it is fundamental to the target.

Or is the subtarget a suitable place to include other properties in addition to the user specified function attributes? Or is there another place such target properties can be defined?

It does seem there are other helper functions that are very similar to this. So the comment here presumably applies to them too?

[kerbowa]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

889–892

Doesn't the parsing of the result have the same problem you are describing?

We had discussed adding a third target feature bit to track the "Default" setting. My concern was that since we are relying on target features to set these properties we shouldn't introduce more target features that break everything if they are modified. Do you know of any concept that can make some target features static and others dynamic?

Still, maybe adding the third feature are using the existing feature parsing is a better solution. Obviously the parsing here could be improved.

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h

65

Yes, and the original plan was to keep the target features the same because the backend shouldn't care. The problem is that we need to enforce compatible features for every function, and in that sense "Default" and "On" have distinct meaning. The difference also matters when generating EFLAGs. We are adding a pass to check compatibility between target features, and "Default" is part of that equation.

For SramEcc "On", "Off", and "Default" will eventually generate different code.

It also makes sense to be consistent across the stack with the concepts in the new target ID.

127

I wasn't sure if this might ever be used with AMDGPUSubtarget or only GCNSubtarget.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h

65

This is the first I've heard of a pass to check compatibility. I hope this is at most a verifier tool

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

504

Subtarget features and function attributes are distinct things. The uses can blur since the subtarget features are present in a function attribute.

Subtarget features are a list of properties for a processor. Whether the hardware supports something is absolutely a distinct subtarget feature. You could make an argument for the "dynamic" setting for xnack/ecc should be an independent function attribute. However, since you can't really separate this for codegen purposes, I think it's a subtarget feature.

Really anywhere checking for specific GPUs is a problem. This pattern is just going to grow the number of places that need to be updated everytime a new target is added. I'd much rather consolidate as much as possible into the subtarget features attached to the processor definition

[scott.linder]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

954

I think this is the first instance where having distinct, weak types for each feature causes issues. I would prefer this be a function, e.g. raw_ostream& operator<<(raw_ostream&, TargetIDSetting) which really needs you to have one, strong type.

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h

71

Not to distract from the more fundamental questions in the review, but can these all be enum class to get stronger type safety and namespacing? Then you can also drop the prefix on the variants, i.e.

enum class SramEccSetting { Default, On, Off, NotSupported };

Also, if we don't need to leverage the types to prevent e.g. passing an SramEccSetting to something expecting an XnackSetting (and I imagine this is the case, as generally everything will just be asking for the setting, not setting/passing it around), then I think we should just have one enum of the four variants that are common to all "TargetID Target Features", for example:

enum class TargetIDSetting { Default, On, Off, NotSupported };
TargetIDSetting getXnackSetting() const;
TargetIDSetting getSramEccSetting() const;

I think of this as more in line with having e.g. a new type for Meters or Miles. For each "unit" in the domain you should have a new type (specifically one which C++ actually type-checks), but you typically don't need/want BoatLengthMeters and CarLengthMeters because you sacrifice having things that deal with just Meters in a general way.

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

504

I'm surprised there aren't other targets with the same issue?

Or is the idea that there should never be any really immutable target features? You should always be able to hack around on them with e.g. llc by using -mattr? It seems like https://llvm.org/doxygen/classllvm_1_1SubtargetFeatures.html#details hints at this with the paragraph (emphasis mine):

Features are encoded as a string of the form "+attr1,+attr2,-attr3,...,+attrN" A comma separates each feature from the next (all lowercase.) Each of the remaining features is prefixed with + or - indicating whether that feature should be enabled or disabled contrary to the cpu specification.

[kerbowa]

Re-add DoesNotSupport features. Use enum class. No fatal error on setting mismatch.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPU.td

839

This shouldn't be on any target definitions

[kerbowa]

llvm/lib/Target/AMDGPU/AMDGPU.td

839

It's intentional in order to keep the current defaults. Targets with FeatureXNACK are ones that used to be default xnack on.

[arsenm]

LGTM with nits, assuming the follow up change fixes some of the remaining hacks

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

900

Indentation

925

Single quotes

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h

788

Early exit if !hasD16LoadStore and avoid querying getSramEccSetting?

[kerbowa]

Rename "Default" to "Any". Address comments.

[kzhuravl]

llvm/lib/Target/AMDGPU/AMDGPU.td

839

Is this correct? 908 supports xnack, but I think it does not have xnack enabled by default?

[kerbowa]

llvm/lib/Target/AMDGPU/AMDGPU.td

839

Before this patch the feature was omitted on the proc definition. Without FeatureDoseNotSupportXNACK the default was Xnack On. This slightly changes the elf test because of the gloablSTI interaction. The codegen is the same as before.

It all becomes irrelevant since we changing the default soon.

[kzhuravl]

LGTM

[kerbowa]

Update, rebase, and merge with change of default patch.

[kerbowa]

Rebase. Move some utility functions to header.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h

307

Don't see why this would be Optional if the query just crashes if it's missing

[kerbowa]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h

307

It's not constructed until "initializeSubtargetDependencies" is called.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h

307

It can just use an invalid target ID value then

[t-tye]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h

307

But optional is a better way of indicating an object does not yet have a value than imposing that the value itself has a distinguished NULL value.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h

307

There's no reason to use Optional here since None is not a meaningful state. This is only an artifact of the initialization order

[t-tye]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h

307

But wouldn't that mean there has to be a default constructor for AMDGPU::IsaInfo::AMDGPUTargetID? Is there such a default constructor? Is there a meaningful value to default construct to?

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h

307

It could just default to Unsupported or Any

[kzhuravl]

Any progress on this? Does it need another rebase?

thanks

cc @t-tye

[kerbowa]

Rebase.

[t-tye]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h

307

But it seems incorrect to default to a value that is not meaningful for the target. So it would need to be determinable whether the target did or did not support the features to know whether to use Unsupported or Any as the default setting. Is that possible at this point of the code, or is that figured out later when this field is given a non-None value?

[kerbowa]

Don't use Optional type for TargetID.

[t-tye]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

80

Is moving constructing TargetID to the constructor of GCNSubtarget equivalent? Was it originally put in GCNSubtarget::initializeSubtargetDependencies because the base classes were modified between the construction of GCNSubtarget and calling its GCNSubtarget::initializeSubtargetDependencies? I know this was all rather contorted logic that made it hard to reason about.

[kerbowa]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

80

I think it is functionally equivalent. The static parts of the FeatureBits that determine whether a feature is supported are constructed early before initializeSubtargetDependencies is called. We can use this to have an initial value of 'Unsupported' or 'Any' for TargetID, and then do more processing based on the feature string when initializeSubtargetDependencies is called later.

[t-tye]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

80

So what does it mean to say "static parts"? Aren't all FeatureBits born equal? They are just bits that can be overridden by function attributes. Is there a function attribute that can change the "isXnackSupported" feature bit? If so what does that mean? Is that legal and should it be diagnosed as not allowed?

[kerbowa]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

80

Yes, I just meant the "static" feature bits before looking at function attributes or doing any other processing. It was always possible to do pathological things like disabling any features for any processor by using function attributes. I'm not aware of any target which tries to prevent this.

[t-tye]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

80

But what if the function attributes change the value of the is*Supported feature bits? You now have inconsistent state between the TargetId and the FeatureBits. But allowing the Is*Supported to be changed by function attributes seems wrong to allow in the first place.

[kerbowa]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

80

I do see your point, but you could say the same about any target feature, e.g. what if someone changes the value of FeatureFP64 with function attributes. You can expect to break things easily this way. No one should be modifying the Is*Supported feature the same way they shouldn't be modifying the FP64 feature.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

135–136

I think having debug printing in the subtarget constructor would be a net annoyance

[kerbowa]

Rebase after 560d7e04113bf.

[kerbowa]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

135–136

We already had debug printouts in the constructor from ParseSubtargetFeatures. Do you think they should all be removed? Right now the output with '-debug-only=amdgpu-subtarget' looks like this, which seems helpful to me.

Features:+promote-alloca,+load-store-opt,+enable-ds128,+enable-prt-strict-null,
CPU:gfx900
TuneCPU:gfx900

xnack setting for subtarget: Any
sramecc setting for subtarget: Unsupported

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

135–136

I forgot about those, so I guess it doesn't matter

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

286

Don't need .hasValue()

[kerbowa]

Rebase. Address comments.

[t-tye]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h

384

Why did this get changed to DoesNotSupportSRAMECC? Same question for bool SupportsXNACK.

[t-tye]

LGTM

[kerbowa]

Rebase.

[kzhuravl]

LGTM

llvm/lib/Target/AMDGPU/AMDGPU.td

class GCNSubtargetFeatureGeneration <string Value,
                                      list<SubtargetFeature> Implies> :
         SubtargetFeatureGeneration <Value, FeatureName, "GCNSubtarget", Implies>;
 
 def FeatureSouthernIslands : GCNSubtargetFeatureGeneration<"SOUTHERN_ISLANDS",
     "southern-islands",
   [FeatureFP64, FeatureLocalMemorySize32768, FeatureMIMG_R128,
   FeatureWavefrontSize64, FeatureSMemTimeInst, FeatureMadMacF32Insts,
   FeatureDsSrc2Insts, FeatureLDSBankCount32, FeatureMovrel,
-  FeatureTrigReducedRange, FeatureDoesNotSupportSRAMECC,
-  FeatureDoesNotSupportXNACK]
+  FeatureTrigReducedRange, FeatureDoesNotSupportXNACK,
+  FeatureDoesNotSupportSRAMECC]
 >;
 
 def FeatureSeaIslands : GCNSubtargetFeatureGeneration<"SEA_ISLANDS",
     "sea-islands",
   [FeatureFP64, FeatureLocalMemorySize65536, FeatureMIMG_R128,
   FeatureWavefrontSize64, FeatureFlatAddressSpace,
   FeatureCIInsts, FeatureMovrel, FeatureTrigReducedRange,
   FeatureGFX7GFX8GFX9Insts, FeatureSMemTimeInst, FeatureMadMacF32Insts,
-  FeatureDsSrc2Insts, FeatureDoesNotSupportSRAMECC]
+  FeatureDsSrc2Insts, FeatureDoesNotSupportXNACK,
+  FeatureDoesNotSupportSRAMECC]
 >;
 
 def FeatureVolcanicIslands : GCNSubtargetFeatureGeneration<"VOLCANIC_ISLANDS",
   "volcanic-islands",
   [FeatureFP64, FeatureLocalMemorySize65536, FeatureMIMG_R128,
    FeatureWavefrontSize64, FeatureFlatAddressSpace,
    FeatureGCN3Encoding, FeatureCIInsts, Feature16BitInsts,
    FeatureSMemRealTime, FeatureVGPRIndexMode, FeatureMovrel,

 class FeatureSet<list<SubtargetFeature> Features_> {
   list<SubtargetFeature> Features = Features_;
 }
 
 def FeatureISAVersion6_0_0 : FeatureSet<[FeatureSouthernIslands,
    FeatureFastFMAF32,
    HalfRate64Ops,
    FeatureLDSBankCount32,
-   FeatureDoesNotSupportXNACK,
    FeatureCodeObjectV3]>;
 
 def FeatureISAVersion6_0_1 : FeatureSet<
   [FeatureSouthernIslands,
    FeatureLDSBankCount32,
-   FeatureDoesNotSupportXNACK,
    FeatureCodeObjectV3]>;
 
 def FeatureISAVersion7_0_0 : FeatureSet<
   [FeatureSeaIslands,
    FeatureLDSBankCount32,
-   FeatureDoesNotSupportXNACK,
    FeatureCodeObjectV3]>;
 
 def FeatureISAVersion7_0_1 : FeatureSet<
   [FeatureSeaIslands,
    HalfRate64Ops,
    FeatureLDSBankCount32,
    FeatureFastFMAF32,
-   FeatureDoesNotSupportXNACK,
    FeatureCodeObjectV3]>;
 
 def FeatureISAVersion7_0_2 : FeatureSet<
   [FeatureSeaIslands,
    FeatureLDSBankCount16,
    FeatureFastFMAF32,
-   FeatureDoesNotSupportXNACK,
    FeatureCodeObjectV3]>;
 
 def FeatureISAVersion7_0_3 : FeatureSet<
   [FeatureSeaIslands,
    FeatureLDSBankCount16,
-   FeatureDoesNotSupportXNACK,
    FeatureCodeObjectV3]>;
 
 def FeatureISAVersion7_0_4 : FeatureSet<
   [FeatureSeaIslands,
    FeatureLDSBankCount32,
-   FeatureDoesNotSupportXNACK,
    FeatureCodeObjectV3]>;
 
 def FeatureISAVersion8_0_1 : FeatureSet<
   [FeatureVolcanicIslands,
    FeatureFastFMAF32,
    HalfRate64Ops,
    FeatureLDSBankCount32,
    FeatureXNACK,
    FeatureUnpackedD16VMem,
    FeatureCodeObjectV3]>;
 
 def FeatureISAVersion8_0_2 : FeatureSet<
   [FeatureVolcanicIslands,
    FeatureLDSBankCount32,
    FeatureSGPRInitBug,
    FeatureUnpackedD16VMem,
-   FeatureDoesNotSupportXNACK,
    FeatureCodeObjectV3]>;
 
 def FeatureISAVersion8_0_3 : FeatureSet<
   [FeatureVolcanicIslands,
    FeatureLDSBankCount32,
    FeatureUnpackedD16VMem,
-   FeatureDoesNotSupportXNACK,
    FeatureCodeObjectV3]>;
 
 def FeatureISAVersion8_1_0 : FeatureSet<
   [FeatureVolcanicIslands,
    FeatureLDSBankCount16,
    FeatureXNACK,
    FeatureCodeObjectV3]>;
 
 def FeatureISAVersion9_0_0 : FeatureSet<
   [FeatureGFX9,
    FeatureMadMixInsts,
    FeatureLDSBankCount32,
    FeatureCodeObjectV3,
-   FeatureDoesNotSupportXNACK,
    FeatureDoesNotSupportSRAMECC]>;
 
 def FeatureISAVersion9_0_2 : FeatureSet<
   [FeatureGFX9,
    FeatureMadMixInsts,
    FeatureLDSBankCount32,
    FeatureXNACK,
    FeatureDoesNotSupportSRAMECC,
    FeatureCodeObjectV3]>;
 
 def FeatureISAVersion9_0_4 : FeatureSet<
   [FeatureGFX9,
    FeatureLDSBankCount32,
    FeatureFmaMixInsts,
-   FeatureDoesNotSupportXNACK,
    FeatureDoesNotSupportSRAMECC,
    FeatureCodeObjectV3]>;
 
 def FeatureISAVersion9_0_6 : FeatureSet<
   [FeatureGFX9,
    HalfRate64Ops,
    FeatureFmaMixInsts,
    FeatureLDSBankCount32,
    FeatureDLInsts,
    FeatureDot1Insts,
    FeatureDot2Insts,
-   FeatureDoesNotSupportXNACK,
    FeatureCodeObjectV3]>;
 
 def FeatureISAVersion9_0_8 : FeatureSet<
   [FeatureGFX9,
    HalfRate64Ops,
    FeatureFmaMixInsts,
    FeatureLDSBankCount32,
    FeatureDLInsts,
    FeatureDot1Insts,
    FeatureDot2Insts,
    FeatureDot3Insts,
    FeatureDot4Insts,
    FeatureDot5Insts,
    FeatureDot6Insts,
    FeatureMAIInsts,
    FeaturePkFmacF16Inst,
    FeatureAtomicFaddInsts,
+   FeatureXNACK,
    FeatureSRAMECC,
    FeatureMFMAInlineLiteralBug,
    FeatureCodeObjectV3]>;
 
 def FeatureISAVersion9_0_9 : FeatureSet<
   [FeatureGFX9,
    FeatureMadMixInsts,
    FeatureLDSBankCount32,
!listconcat(FeatureGroup.GFX10_1_Bugs,
      FeatureScalarStores,
      FeatureScalarAtomics,
      FeatureScalarFlatScratchInsts,
      FeatureGetWaveIdInst,
      FeatureSMemTimeInst,
      FeatureMadMacF32Insts,
      FeatureDsSrc2Insts,
      FeatureLdsMisalignedBug,
-     FeatureDoesNotSupportXNACK,
      FeatureCodeObjectV3])>;
 
 def FeatureISAVersion10_1_1 : FeatureSet<
   !listconcat(FeatureGroup.GFX10_1_Bugs,
     [FeatureGFX10,
      FeatureLDSBankCount32,
      FeatureDLInsts,
      FeatureDot1Insts,
      FeatureDot2Insts,
      FeatureDot5Insts,
      FeatureDot6Insts,
      FeatureNSAEncoding,
      FeatureWavefrontSize32,
      FeatureScalarStores,
      FeatureScalarAtomics,
      FeatureScalarFlatScratchInsts,
      FeatureGetWaveIdInst,
      FeatureSMemTimeInst,
      FeatureMadMacF32Insts,
      FeatureDsSrc2Insts,
-     FeatureDoesNotSupportXNACK,
      FeatureCodeObjectV3])>;
 
 def FeatureISAVersion10_1_2 : FeatureSet<
   !listconcat(FeatureGroup.GFX10_1_Bugs,
     [FeatureGFX10,
      FeatureLDSBankCount32,
      FeatureDLInsts,
      FeatureDot1Insts,
      FeatureDot2Insts,
      FeatureDot5Insts,
      FeatureDot6Insts,
      FeatureNSAEncoding,
      FeatureWavefrontSize32,
      FeatureScalarStores,
      FeatureScalarAtomics,
      FeatureScalarFlatScratchInsts,
      FeatureGetWaveIdInst,
      FeatureSMemTimeInst,
      FeatureMadMacF32Insts,
      FeatureDsSrc2Insts,
      FeatureLdsMisalignedBug,
-     FeatureDoesNotSupportXNACK,
      FeatureCodeObjectV3])>;
 
 def FeatureISAVersion10_3_0 : FeatureSet<
   [FeatureGFX10,
    FeatureGFX10_BEncoding,
    FeatureGFX10_3Insts,
    FeatureLDSBankCount32,
    FeatureDLInsts,
    FeatureDot1Insts,
    FeatureDot2Insts,
    FeatureDot5Insts,
    FeatureDot6Insts,
    FeatureNSAEncoding,
    FeatureWavefrontSize32,
-   FeatureDoesNotSupportXNACK,
    FeatureCodeObjectV3]>;
 
 //===----------------------------------------------------------------------===//
 
 def AMDGPUInstrInfo : InstrInfo {
   let guessInstructionProperties = 1;
   let noNamedPositionallyEncodedOperands = 1;
 }

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h

enum Generation {
     SOUTHERN_ISLANDS = 4,
     SEA_ISLANDS = 5,
     VOLCANIC_ISLANDS = 6,
     GFX9 = 7,
     GFX10 = 8
   };
 
 private:
   Triple TargetTriple;
 
 protected:
   bool Has16BitInsts;
   bool HasMadMixInsts;
   bool HasMadMacF32Insts;
   bool HasDsSrc2Insts;
   bool HasSDWA;
   bool HasVOP3PInsts;
   bool HasMulI24;
   bool HasMulU24;
   bool HasInv2PiInlineImm;
   bool HasFminFmaxLegacy;
   bool EnablePromoteAlloca;
   bool HasTrigReducedRange;
public:
   /// Inverse of getMaxLocalMemWithWaveCount. Return the maximum wavecount if
   /// the given LDS memory size is the only constraint.
   unsigned getOccupancyWithLocalMemSize(uint32_t Bytes, const Function &) const;
 
   unsigned getOccupancyWithLocalMemSize(const MachineFunction &MF) const;
 
   bool isAmdHsaOS() const {
     return TargetTriple.getOS() == Triple::AMDHSA;
   }
 
   bool isAmdPalOS() const {
     return TargetTriple.getOS() == Triple::AMDPAL;
   }
 
   bool isMesa3DOS() const {
     return TargetTriple.getOS() == Triple::Mesa3D;
   }
enum TrapID {
     TrapIDDebugReservedFE = 0xfe,
     TrapIDDebugReservedFF = 0xff
   };
 
   enum TrapRegValues {
     LLVMTrapHandlerRegValue = 1
   };
 
+  /// Returns the current Xnack TargetIDSetting, possible options are
+  /// "NotSupported", "Any", "Off", and "On".
+  AMDGPU::IsaInfo::TargetIDSetting getXnackSetting() const;
+
+  /// Returns the current SramEcc TargetIDSetting, possible options are
+  /// "NotSupported", "Any", "Off", and "On".
+  AMDGPU::IsaInfo::TargetIDSetting getSramEccSetting() const;
+
 private:
   /// GlobalISel related APIs.
   std::unique_ptr<AMDGPUCallLowering> CallLoweringInfo;
   std::unique_ptr<InlineAsmLowering> InlineAsmLoweringInfo;
   std::unique_ptr<InstructionSelector> InstSelector;
   std::unique_ptr<LegalizerInfo> Legalizer;
   std::unique_ptr<RegisterBankInfo> RegBankInfo;
 
 protected:
   // Basic subtarget description.
   Triple TargetTriple;
   unsigned Gen;
   InstrItineraryData InstrItins;
   int LDSBankCount;
   unsigned MaxPrivateElementSize;
 
   // Possibly statically set by tablegen, but may want to be overridden.
protected:
 
   // Used as options.
   bool EnableLoadStoreOpt;
   bool EnableUnsafeDSOffsetFolding;
   bool EnableSIScheduler;
   bool EnableDS128;
   bool EnablePRTStrictNull;
   bool DumpCode;
+  bool SupportAnyXnackSetting;
+  bool SupportAnySramEccSetting;
 
   // Subtarget statically properties set by tablegen
   bool FP64;
   bool FMA;
   bool MIMG_R128;
   bool IsGCN;
   bool GCN3Encoding;
   bool CIInsts;
protected:
   bool HasDot2Insts;
   bool HasDot3Insts;
   bool HasDot4Insts;
   bool HasDot5Insts;
   bool HasDot6Insts;
   bool HasMAIInsts;
   bool HasPkFmacF16Inst;
   bool HasAtomicFaddInsts;
   bool EnableSRAMECC;
   bool DoesNotSupportSRAMECC;
   bool HasNoSdstCMPX;
   bool HasVscnt;
   bool HasGetWaveIdInst;
   bool HasSMemTimeInst;
   bool HasRegisterBanking;
   bool HasVOP3Literal;
   bool HasNoDataDepHazard;

 private:
   SIInstrInfo InstrInfo;
   SITargetLowering TLInfo;
   SIFrameLowering FrameLowering;
 
   // See COMPUTE_TMPRING_SIZE.WAVESIZE, 13-bit field in units of 256-dword.
   static const unsigned MaxWaveScratchSize = (256 * 4) * ((1 << 13) - 1);
 
+  // Initialize Xnack and SramEcc settings based on subtarget support and
+  // requested features.
+  void initializeXnackAndSramEcc(StringRef FS, StringRef GPU);
+
 public:
   GCNSubtarget(const Triple &TT, StringRef GPU, StringRef FS,
                const GCNTargetMachine &TM);
   ~GCNSubtarget() override;
 
   GCNSubtarget &initializeSubtargetDependencies(const Triple &TT,
                                                    StringRef GPU, StringRef FS);
 
bool hasApertureRegs() const {
     return HasApertureRegs;
   }
 
   bool isTrapHandlerEnabled() const {
     return TrapHandler;
   }
 
   bool isXNACKEnabled() const {
-    return EnableXNACK;
+    // FIXME: XNACK should be enabled with "Any" as well as "On". We
+    // can then remove this function and start using getXnackSetting directly.
+    return getXnackSetting() == AMDGPU::IsaInfo::TargetIDSetting::On;
+  }
+
+  bool supportAnyXnackSetting() const {
+    return SupportAnyXnackSetting;
   }
 
   bool isCuModeEnabled() const {
     return EnableCuMode;
   }
 
   bool hasFlatAddressSpace() const {
     return FlatAddressSpace;
bool hasFlatLgkmVMemCountInOrder() const {
     return getGeneration() > GFX9;
   }
 
   bool hasD16LoadStore() const {
     return getGeneration() >= GFX9;
   }
 
   bool d16PreservesUnusedBits() const {
-    return hasD16LoadStore() && !isSRAMECCEnabled();
+    if (!hasD16LoadStore())
+      return false;
+    AMDGPU::IsaInfo::TargetIDSetting Setting = getSramEccSetting();
+    return Setting == AMDGPU::IsaInfo::TargetIDSetting::Off ||
+           Setting == AMDGPU::IsaInfo::TargetIDSetting::NotSupported;
   }
 
   bool hasD16Images() const {
     return getGeneration() >= VOLCANIC_ISLANDS;
   }
 
   /// Return if most LDS instructions have an m0 use that require m0 to be
   /// iniitalized.
public:
   bool hasPkFmacF16Inst() const {
     return HasPkFmacF16Inst;
   }
 
   bool hasAtomicFaddInsts() const {
     return HasAtomicFaddInsts;
   }
 
-  bool isSRAMECCEnabled() const {
-    return EnableSRAMECC;
+  bool supportAnySramEccSetting() const {
+    return SupportAnySramEccSetting;
   }
 
   bool hasNoSdstCMPX() const {
     return HasNoSdstCMPX;
   }
 
   bool hasVscnt() const {
     return HasVscnt;

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

 //===-- AMDGPUSubtarget.cpp - AMDGPU Subtarget Information ----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 /// \file
 /// Implements the AMDGPU specific subclass of TargetSubtarget.
 //
 //===----------------------------------------------------------------------===//
 
 #include "AMDGPUSubtarget.h"
 #include "AMDGPU.h"
-#include "AMDGPUTargetMachine.h"
 #include "AMDGPUCallLowering.h"
 #include "AMDGPUInstructionSelector.h"
 #include "AMDGPULegalizerInfo.h"
 #include "AMDGPURegisterBankInfo.h"
-#include "SIMachineFunctionInfo.h"
+#include "AMDGPUTargetMachine.h"
 #include "MCTargetDesc/AMDGPUMCTargetDesc.h"
+#include "SIMachineFunctionInfo.h"
+#include "Utils/AMDGPUBaseInfo.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/CodeGen/MachineScheduler.h"
-#include "llvm/MC/MCSubtargetInfo.h"
-#include "llvm/IR/MDBuilder.h"
 #include "llvm/CodeGen/TargetFrameLowering.h"
+#include "llvm/IR/MDBuilder.h"
+#include "llvm/MC/MCSubtargetInfo.h"
 #include <algorithm>
 
 using namespace llvm;
 
 #define DEBUG_TYPE "amdgpu-subtarget"
 
 #define GET_SUBTARGETINFO_TARGET_DESC
 #define GET_SUBTARGETINFO_CTOR
GCNSubtarget::initializeSubtargetDependencies(const Triple &TT,
   // Determine default and user-specified characteristics
   //
   // We want to be able to turn these off, but making this a subtarget feature
   // for SI has the unhelpful behavior that it unsets everything else if you
   // disable it.
   //
   // Similarly we want enable-prt-strict-null to be on by default and not to
   // unset everything else if it is disabled
 
   // Assuming ECC is enabled is the conservative default.
-  SmallString<256> FullFS("+promote-alloca,+load-store-opt,+enable-ds128,+sram-ecc,+xnack,");
+  SmallString<256> FullFS("+promote-alloca,+load-store-opt,+enable-ds128,");
 
   if (isAmdHsaOS()) // Turn on FlatForGlobal for HSA.
     FullFS += "+flat-for-global,+unaligned-buffer-access,+trap-handler,";
 
   FullFS += "+enable-prt-strict-null,"; // This is overridden by a disable in FS
 
   // Disable mutually exclusive bits.
   if (FS.find_lower("+wavefrontsize") != StringRef::npos) {
GCNSubtarget::initializeSubtargetDependencies(const Triple &TT,
   }
 
   // Don't crash on invalid devices.
   if (WavefrontSizeLog2 == 0)
     WavefrontSizeLog2 = 5;
 
   HasFminFmaxLegacy = getGeneration() < AMDGPUSubtarget::VOLCANIC_ISLANDS;
 
-  // Disable XNACK on targets where it is not enabled by default unless it is
-  // explicitly requested.
-  if (!FS.contains("+xnack") && DoesNotSupportXNACK && EnableXNACK) {
-    ToggleFeature(AMDGPU::FeatureXNACK);
-    EnableXNACK = false;
-  }
-
-  // ECC is on by default, but turn it off if the hardware doesn't support it
-  // anyway. This matters for the gfx9 targets with d16 loads, but don't support
-  // ECC.
-  if (DoesNotSupportSRAMECC && EnableSRAMECC) {
-    ToggleFeature(AMDGPU::FeatureSRAMECC);
-    EnableSRAMECC = false;
-  }
+  initializeXnackAndSramEcc(FS, GPU);
 
   return *this;
 }
 
 AMDGPUSubtarget::AMDGPUSubtarget(const Triple &TT) :
   TargetTriple(TT),
   Has16BitInsts(false),
   HasMadMixInsts(false),
   HasMadMacF32Insts(false),
   HasDsSrc2Insts(false),
   HasSDWA(false),
   HasVOP3PInsts(false),
   HasMulI24(true),
   HasMulU24(true),
   HasInv2PiInlineImm(false),
   HasFminFmaxLegacy(true),
   EnablePromoteAlloca(false),
   HasTrigReducedRange(false),
   MaxWavesPerEU(10),
   LocalMemorySize(0),
   WavefrontSizeLog2(0)
   { }
 
 GCNSubtarget::GCNSubtarget(const Triple &TT, StringRef GPU, StringRef FS,
                            const GCNTargetMachine &TM) :
     AMDGPUGenSubtargetInfo(TT, GPU, FS),
     AMDGPUSubtarget(TT),
     TargetTriple(TT),
     Gen(TT.getOS() == Triple::AMDHSA ? SEA_ISLANDS : SOUTHERN_ISLANDS),
     InstrItins(getInstrItineraryForCPU(GPU)),
     LDSBankCount(0),
     MaxPrivateElementSize(0),
 
GCNSubtarget::GCNSubtarget(const Triple &TT, StringRef GPU, StringRef FS,
 
     EnableLoadStoreOpt(false),
     EnableUnsafeDSOffsetFolding(false),
     EnableSIScheduler(false),
     EnableDS128(false),
     EnablePRTStrictNull(false),
     DumpCode(false),
 
+    // Initialize with the conservative defaults.
+    SupportAnyXnackSetting(true),
+    SupportAnySramEccSetting(true),
+
     FP64(false),
     GCN3Encoding(false),
     CIInsts(false),
     GFX8Insts(false),
     GFX9Insts(false),
     GFX10Insts(false),
     GFX10_3Insts(false),
     GFX7GFX8GFX9Insts(false),

 };
 } // namespace
 
 void GCNSubtarget::getPostRAMutations(
     std::vector<std::unique_ptr<ScheduleDAGMutation>> &Mutations) const {
   Mutations.push_back(std::make_unique<FillMFMAShadowMutation>(&InstrInfo));
 }
 
+void GCNSubtarget::initializeXnackAndSramEcc(StringRef FS, StringRef GPU) {
+  bool XnackOnSupportRequested = FS.contains("+xnack");
+  bool SramEccOnSupportRequested = FS.contains("+sram-ecc");
+  bool XnackOffSupportRequested = FS.contains("-xnack");
+  bool SramEccOffSupportRequested = FS.contains("-sram-ecc");
+
+  // Check if support for XNACK or SRAMECC is explicitly enabled or disabled.
+  // In the absence of the target features we assume we must produce code that
+  // can run in any environment.
+  SupportAnyXnackSetting =
+      !(XnackOnSupportRequested || XnackOffSupportRequested);
+  SupportAnySramEccSetting =
+      !(SramEccOnSupportRequested || SramEccOffSupportRequested);
+
+  if (XnackOnSupportRequested && DoesNotSupportXNACK)
+    errs() << "warning: Xnack On was requested for a processor that does not "
+              "support it!\n";
+
+  if (SramEccOnSupportRequested && DoesNotSupportSRAMECC)
+    errs() << "warning: SramEcc On was requested for a processor that does not "
+              "support it!\n";
+
+  if (XnackOffSupportRequested && DoesNotSupportXNACK)
+    errs() << "warning: Xnack Off was requested for a processor that does not "
+              "support it!\n";
+
+  if (SramEccOffSupportRequested && DoesNotSupportSRAMECC)
+    errs() << "warning: SramEcc Off was requested for a processor that does "
+              "not support it!\n";
+
+  // FIXME: These hacks are necessary to support backwards compatibility with
+  // the old defaults for xnack. When the new targetid feature is enabled this,
+  // along with the change in isXNACKEnabled can be updated to reflect the true
+  // intended meaning of "default" for these settings.
+  if (EnableXNACK)
+    SupportAnyXnackSetting = false;
+  if (GPU == "generic" || GPU == "generic-hsa") {
+    SupportAnySramEccSetting = true;
+
+    // FIXME
+    SupportAnyXnackSetting = false;
+    EnableXNACK = true;
+  }
+
+  LLVM_DEBUG(dbgs() << "XNACK setting for subtarget: " << getXnackSetting()
+                    << '\n');
+  LLVM_DEBUG(dbgs() << "SRAMECC setting for subtarget: " << getSramEccSetting()
+                    << '\n');
+}
+
+AMDGPU::IsaInfo::TargetIDSetting GCNSubtarget::getXnackSetting() const {
+  if (DoesNotSupportXNACK)
+    return AMDGPU::IsaInfo::TargetIDSetting::NotSupported;
+
+  if (supportAnyXnackSetting())
+    return AMDGPU::IsaInfo::TargetIDSetting::Any;
+
+  if (!EnableXNACK)
+    return AMDGPU::IsaInfo::TargetIDSetting::Off;
+  return AMDGPU::IsaInfo::TargetIDSetting::On;
+}
+
+AMDGPU::IsaInfo::TargetIDSetting GCNSubtarget::getSramEccSetting() const {
+  if (DoesNotSupportSRAMECC)
+    return AMDGPU::IsaInfo::TargetIDSetting::NotSupported;
+
+  if (supportAnySramEccSetting())
+    return AMDGPU::IsaInfo::TargetIDSetting::Any;
+
+  if (!EnableSRAMECC)
+    return AMDGPU::IsaInfo::TargetIDSetting::Off;
+  return AMDGPU::IsaInfo::TargetIDSetting::On;
+}
+
 const AMDGPUSubtarget &AMDGPUSubtarget::get(const MachineFunction &MF) {
   if (MF.getTarget().getTargetTriple().getArch() == Triple::amdgcn)
     return static_cast<const AMDGPUSubtarget&>(MF.getSubtarget<GCNSubtarget>());
   else
     return static_cast<const AMDGPUSubtarget&>(MF.getSubtarget<R600Subtarget>());
 }
 
 const AMDGPUSubtarget &AMDGPUSubtarget::get(const TargetMachine &TM, const Function &F) {
   if (TM.getTargetTriple().getArch() == Triple::amdgcn)
     return static_cast<const AMDGPUSubtarget&>(TM.getSubtarget<GCNSubtarget>(F));
   else
     return static_cast<const AMDGPUSubtarget&>(TM.getSubtarget<R600Subtarget>(F));
 }

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h

 #include "SIDefines.h"
 #include "llvm/IR/CallingConv.h"
 #include "llvm/MC/MCInstrDesc.h"
 #include "llvm/Support/AMDHSAKernelDescriptor.h"
 #include "llvm/Support/Alignment.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/TargetParser.h"
+#include "llvm/Support/raw_ostream.h"
 #include <cstdint>
 #include <string>
 #include <utility>
 
 namespace llvm {
 
 class Argument;
 class Function;

 
 enum {
   // The closed Vulkan driver sets 96, which limits the wave count to 8 but
   // doesn't spill SGPRs as much as when 80 is set.
   FIXED_NUM_SGPRS_FOR_INIT_BUG = 96,
   TRAP_NUM_SGPRS = 16
 };
 
+enum class TargetIDSetting {
+  NotSupported,
+  Any,
+  Off,
+  On
+};
+
 /// Streams isa version string for given subtarget \p STI into \p Stream.
 void streamIsaVersion(const MCSubtargetInfo *STI, raw_ostream &Stream);
 
 /// \returns True if given subtarget \p STI supports code object version 3,
 /// false otherwise.
 bool hasCodeObjectV3(const MCSubtargetInfo *STI);
 
 /// \returns Wavefront size for given subtarget \p STI.
bool isInlineCompatible(SIModeRegisterDefaults CalleeMode) const {
     return oneWayCompatible(FP64FP16InputDenormals, CalleeMode.FP64FP16InputDenormals) &&
            oneWayCompatible(FP64FP16OutputDenormals, CalleeMode.FP64FP16OutputDenormals) &&
            oneWayCompatible(FP32InputDenormals, CalleeMode.FP32InputDenormals) &&
            oneWayCompatible(FP32OutputDenormals, CalleeMode.FP32OutputDenormals);
   }
 };
 
 } // end namespace AMDGPU
+
+raw_ostream &operator<<(raw_ostream &OS,
+                        const AMDGPU::IsaInfo::TargetIDSetting S);
+
 } // end namespace llvm
 
 #endif // LLVM_LIB_TARGET_AMDGPU_UTILS_AMDGPUBASEINFO_H

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

if (STI->getFeatureBits().test(FeatureLocalMemorySize65536))
     return 65536;
 
   return 0;
 }
 
 unsigned getEUsPerCU(const MCSubtargetInfo *STI) {
   // "Per CU" really means "per whatever functional block the waves of a
   // workgroup must share". For gfx10 in CU mode this is the CU, which contains
   // two SIMDs.
   if (isGFX10(*STI) && STI->getFeatureBits().test(FeatureCuMode))
     return 2;
   // Pre-gfx10 a CU contains four SIMDs. For gfx10 in WGP mode the WGP contains
   // two CUs, so a total of four SIMDs.
   return 4;
 }
 
 unsigned getMaxWorkGroupsPerCU(const MCSubtargetInfo *STI,
unsigned getNumVGPRBlocks(const MCSubtargetInfo *STI, unsigned NumVGPRs,
                           Optional<bool> EnableWavefrontSize32) {
   NumVGPRs = alignTo(std::max(1u, NumVGPRs),
                      getVGPREncodingGranule(STI, EnableWavefrontSize32));
   // VGPRBlocks is actual number of VGPR blocks minus 1.
   return NumVGPRs / getVGPREncodingGranule(STI, EnableWavefrontSize32) - 1;
 }
 
 } // end namespace IsaInfo
 
 void initDefaultAMDKernelCodeT(amd_kernel_code_t &Header,
                                const MCSubtargetInfo *STI) {
   IsaVersion Version = getIsaVersion(STI->getCPU());
 
   memset(&Header, 0, sizeof(Header));
 
   Header.amd_kernel_code_version_major = 1;
   Header.amd_kernel_code_version_minor = 2;
   Header.amd_machine_kind = 1; // AMD_MACHINE_KIND_AMDGPU
   Header.amd_machine_version_major = Version.Major;
   Header.amd_machine_version_minor = Version.Minor;
   Header.amd_machine_version_stepping = Version.Stepping;
   Header.kernel_code_entry_byte_offset = sizeof(Header);
   Header.wavefront_size = 6;
 
   // If the code object does not support indirect functions, then the value must
   // be 0xffffffff.
   Header.call_convention = -1;
 
   // These alignment values are specified in powers of two, so alignment =
   // 2^n.  The minimum alignment is 2^4 = 16.
   Header.kernarg_segment_alignment = 4;
   Header.group_segment_alignment = 4;
   Header.private_segment_alignment = 4;
 
   if (Version.Major >= 10) {
     if (STI->getFeatureBits().test(FeatureWavefrontSize32)) {
       Header.wavefront_size = 5;
       Header.code_properties |= AMD_CODE_PROPERTY_ENABLE_WAVEFRONT_SIZE32;
     }
     Header.compute_pgm_resource_registers |=
       S_00B848_WGP_MODE(STI->getFeatureBits().test(FeatureCuMode) ? 0 : 1) |
       S_00B848_MEM_ORDERED(1);
   }
 }
 
 amdhsa::kernel_descriptor_t getDefaultAmdhsaKernelDescriptor(

 
 const GcnBufferFormatInfo *getGcnBufferFormatInfo(uint8_t Format,
                                                   const MCSubtargetInfo &STI) {
   return isGFX10(STI) ? getGfx10PlusBufferFormatInfo(Format)
                       : getGfx9BufferFormatInfo(Format);
 }
 
 } // namespace AMDGPU
+
+raw_ostream &operator<<(raw_ostream &OS,
+                        const AMDGPU::IsaInfo::TargetIDSetting S) {
+  switch (S) {
+  case (AMDGPU::IsaInfo::TargetIDSetting::NotSupported):
+    OS << "Not Supported";
+    break;
+  case (AMDGPU::IsaInfo::TargetIDSetting::Any):
+    OS << "Any";
+    break;
+  case (AMDGPU::IsaInfo::TargetIDSetting::Off):
+    OS << "Off";
+    break;
+  case (AMDGPU::IsaInfo::TargetIDSetting::On):
+    OS << "On";
+    break;
+  }
+  return OS;
+}
+
 } // namespace llvm

llvm/test/CodeGen/AMDGPU/elf-header-flags-sram-ecc.ll

-; RUN: llc -filetype=obj -march=amdgcn -mcpu=gfx902 < %s | llvm-readobj --file-headers - | FileCheck --check-prefixes=NO-SRAM-ECC-GFX902 %s
-; RUN: llc -filetype=obj -march=amdgcn -mcpu=gfx902 -mattr=-sram-ecc < %s | llvm-readobj --file-headers - | FileCheck --check-prefixes=NO-SRAM-ECC-GFX902 %s
-; RUN: llc -filetype=obj -march=amdgcn -mcpu=gfx902 -mattr=+sram-ecc < %s | llvm-readobj --file-headers - | FileCheck --check-prefixes=SRAM-ECC-GFX902 %s
-
 ; RUN: llc -filetype=obj -march=amdgcn -mcpu=gfx906 < %s | llvm-readobj --file-headers - | FileCheck --check-prefixes=NO-SRAM-ECC-GFX906 %s
 ; RUN: llc -filetype=obj -march=amdgcn -mcpu=gfx906 -mattr=-sram-ecc < %s | llvm-readobj --file-headers - | FileCheck --check-prefixes=NO-SRAM-ECC-GFX906 %s
 ; RUN: llc -filetype=obj -march=amdgcn -mcpu=gfx906 -mattr=+sram-ecc < %s | llvm-readobj --file-headers - | FileCheck --check-prefixes=SRAM-ECC-GFX906 %s
 ; RUN: llc -filetype=obj -march=amdgcn -mcpu=gfx906 -mattr=+sram-ecc,+xnack < %s | llvm-readobj --file-headers - | FileCheck --check-prefixes=SRAM-ECC-XNACK-GFX906 %s
 
 ; RUN: llc -filetype=obj -march=amdgcn -mcpu=gfx908 < %s | llvm-readobj -file-headers - | FileCheck --check-prefix=SRAM-ECC-GFX908 %s
 
-; NO-SRAM-ECC-GFX902:      Flags [
-; NO-SRAM-ECC-GFX902-NEXT:   EF_AMDGPU_MACH_AMDGCN_GFX902 (0x2D)
-; NO-SRAM-ECC-GFX902-NEXT:   EF_AMDGPU_XNACK              (0x100)
-; NO-SRAM-ECC-GFX902-NEXT: ]
-
-; SRAM-ECC-GFX902:      Flags [
-; SRAM-ECC-GFX902-NEXT:   EF_AMDGPU_MACH_AMDGCN_GFX902 (0x2D)
-; SRAM-ECC-GFX902-NEXT:   EF_AMDGPU_SRAM_ECC           (0x200)
-; SRAM-ECC-GFX902-NEXT:   EF_AMDGPU_XNACK              (0x100)
-; SRAM-ECC-GFX902-NEXT: ]
-
 ; NO-SRAM-ECC-GFX906:      Flags [
 ; NO-SRAM-ECC-GFX906-NEXT:   EF_AMDGPU_MACH_AMDGCN_GFX906 (0x2F)
 ; NO-SRAM-ECC-GFX906-NEXT: ]
 
 ; SRAM-ECC-GFX906:      Flags [
 ; SRAM-ECC-GFX906-NEXT:   EF_AMDGPU_MACH_AMDGCN_GFX906 (0x2F)
 ; SRAM-ECC-GFX906-NEXT:   EF_AMDGPU_SRAM_ECC           (0x200)
 ; SRAM-ECC-GFX906-NEXT: ]
 
 ; SRAM-ECC-XNACK-GFX906:      Flags [
 ; SRAM-ECC-XNACK-GFX906-NEXT:   EF_AMDGPU_MACH_AMDGCN_GFX906 (0x2F)
 ; SRAM-ECC-XNACK-GFX906-NEXT:   EF_AMDGPU_SRAM_ECC           (0x200)
 ; SRAM-ECC-XNACK-GFX906-NEXT:   EF_AMDGPU_XNACK              (0x100)
 ; SRAM-ECC-XNACK-GFX906-NEXT: ]
 
-; SRAM-ECC-GFX908: Flags [ (0x230)
+; SRAM-ECC-GFX908: Flags [
 ; SRAM-ECC-GFX908:    EF_AMDGPU_MACH_AMDGCN_GFX908 (0x30)
-; SRAM-ECC-GFX908:    EF_AMDGPU_SRAM_ECC (0x200)
+; SRAM-ECC-GFX908:    EF_AMDGPU_SRAM_ECC           (0x200)
+; SRAM-ECC-GFX908:    EF_AMDGPU_XNACK              (0x100)
 ; SRAM-ECC-GFX908:  ]
 
 define amdgpu_kernel void @elf_header() {
   ret void
 }

llvm/test/CodeGen/AMDGPU/sramecc-subtarget-feature-any.ll

+; RUN: llc -march=amdgcn -mcpu=gfx700 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=NOT-SUPPORTED %s
+; RUN: llc -march=amdgcn -mcpu=gfx906 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=ANY %s
+; RUN: llc -march=amdgcn -mcpu=gfx908 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=ANY %s
+
+; REQUIRES: asserts
+
+; NOT-SUPPORTED: SRAMECC setting for subtarget: Not Supported
+; ANY: SRAMECC setting for subtarget: Any
+define void @sramecc-subtarget-feature-default() #0 {
+  ret void
+}
+
+attributes #0 = { nounwind }

llvm/test/CodeGen/AMDGPU/sramecc-subtarget-feature-disabled.ll

+; RUN: llc -march=amdgcn -mcpu=gfx700 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=WARN %s
+; RUN: llc -march=amdgcn -mcpu=gfx906 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=OFF %s
+; RUN: llc -march=amdgcn -mcpu=gfx908 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=OFF %s
+
+; REQUIRES: asserts
+
+; WARN: warning: SramEcc Off was requested for a processor that does not support it!
+; OFF: SRAMECC setting for subtarget: Off
+
+define void @sramecc-subtarget-feature-disabled() #0 {
+  ret void
+}
+
+attributes #0 = { "target-features"="-sram-ecc" }

llvm/test/CodeGen/AMDGPU/sramecc-subtarget-feature-enabled.ll

+; RUN: llc -march=amdgcn -mcpu=gfx700 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=WARN %s
+; RUN: llc -march=amdgcn -mcpu=gfx906 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=ON %s
+; RUN: llc -march=amdgcn -mcpu=gfx908 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=ON %s
+
+; REQUIRES: asserts
+
+; WARN: warning: SramEcc On was requested for a processor that does not support it!
+; ON: SRAMECC setting for subtarget: On
+define void @sramecc-subtarget-feature-enabled() #0 {
+  ret void
+}
+
+attributes #0 = { "target-features"="+sram-ecc" }

llvm/test/CodeGen/AMDGPU/xnack-subtarget-feature-any.ll

+; RUN: llc -march=amdgcn -mcpu=gfx600 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=NOT-SUPPORTED %s
+; RUN: llc -march=amdgcn -mcpu=gfx700 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=NOT-SUPPORTED %s
+; RUN: llc -march=amdgcn -mcpu=gfx802 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=ANY %s
+; RUN: llc -march=amdgcn -mcpu=gfx900 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=ANY %s
+; RUN: llc -march=amdgcn -mcpu=gfx902 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=ON %s
+; RUN: llc -march=amdgcn -mcpu=gfx1010 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=ANY %s
+
+; REQUIRES: asserts
+
+; Some subtargets have a default setting of 'On' instead of 'Any' to maintain
+; backwards compatibility. This is a temporary measure until the new TargetID is
+; implemented.
+
+; NOT-SUPPORTED: XNACK setting for subtarget: Not Supported
+; ANY: XNACK setting for subtarget: Any
+; ON: XNACK setting for subtarget: On
+define void @xnack-subtarget-feature-any() #0 {
+  ret void
+}
+
+attributes #0 = { nounwind }

llvm/test/CodeGen/AMDGPU/xnack-subtarget-feature-disabled.ll

+; RUN: llc -march=amdgcn -mcpu=gfx600 -debug-only=amdgpu-subtarget -o /dev/null %s 2>&1 | FileCheck --check-prefix=WARN %s
+; RUN: llc -march=amdgcn -mcpu=gfx700 -debug-only=amdgpu-subtarget -o /dev/null %s 2>&1 | FileCheck --check-prefix=WARN %s
+; RUN: llc -march=amdgcn -mcpu=gfx802 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=OFF %s
+; RUN: llc -march=amdgcn -mcpu=gfx900 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=OFF %s
+; RUN: llc -march=amdgcn -mcpu=gfx906 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=OFF %s
+; RUN: llc -march=amdgcn -mcpu=gfx1010 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=OFF %s
+
+; REQUIRES: asserts
+
+; WARN: warning: Xnack Off was requested for a processor that does not support it!
+; OFF: XNACK setting for subtarget: Off
+
+define void @xnack-subtarget-feature-disabled() #0 {
+  ret void
+}
+
+attributes #0 = { "target-features"="-xnack" }

llvm/test/CodeGen/AMDGPU/xnack-subtarget-feature-enabled.ll

+; RUN: llc -march=amdgcn -mcpu=gfx600 -debug-only=amdgpu-subtarget -o /dev/null %s 2>&1 | FileCheck --check-prefix=WARN %s
+; RUN: llc -march=amdgcn -mcpu=gfx700 -debug-only=amdgpu-subtarget -o /dev/null %s 2>&1 | FileCheck --check-prefix=WARN %s
+; RUN: llc -march=amdgcn -mcpu=gfx802 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=ON %s
+; RUN: llc -march=amdgcn -mcpu=gfx900 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=ON %s
+; RUN: llc -march=amdgcn -mcpu=gfx906 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=ON %s
+; RUN: llc -march=amdgcn -mcpu=gfx1010 -debug-only=amdgpu-subtarget -o - %s 2>&1 | FileCheck --check-prefix=ON %s
+
+; REQUIRES: asserts
+
+; WARN: warning: Xnack On was requested for a processor that does not support it!
+; ON: XNACK setting for subtarget: On
+define void @xnack-subtarget-feature-enabled() #0 {
+  ret void
+}
+
+attributes #0 = { "target-features"="+xnack" }