AMDGPU: Assume xnack is enabled by default

[arsenm]

Summary

This is the conservatively correct default. It is always safe to
assume xnack is enabled, but not the converse.

Introduce a feature to blacklist targets where xnack can never be
meaningfully enabled. I'm not sure the targets this is applied to is
100% correct.

[rampitec]

Why do we have both xnack and no-xnack? I thought we are going to drop xnack feature in favor of no-xnack.

[arsenm]

In D61901#1501695, @rampitec wrote:

Why do we have both xnack and no-xnack? I thought we are going to drop xnack feature in favor of no-xnack.

No, my intention is to avoid having no- features, and just use +/- on the positive feature name. The new feature isn't intended for any frontends to set, and is only for marking targets in the backend where the option doesn't do anything.

[rampitec]

LGTM.

We need to emphasize this change also enables xnack by default on all other targets and probably need to be complimented with FE/RT changes.

[arsenm]

r360903

[t-tye]

If this is changing the user visible defaults then the processor table in AMDGPUUsage also needs to be updated and the change of compiler needs to be mentioned in the release notes.

Only targets that do not support XNACK should be marked as DoesNotSupportXNACK which means all gfx8/9/10 should not have DoesNotSupportXNACK as they do support XNACK (as defined in the processor table of AMDGPUUsage).

If an xnack option is given on a target that doe not support XNACK it would be helpful to give a error to the user and not silently ignore the request. In particular it should not cause code object to be generated that does not have XNACK code in it yet is marked in the EFLAGS header that it does have XNACK.

lib/Target/AMDGPU/AMDGPU.td

 >;
 
 def FeatureFmaMixInsts : SubtargetFeature<"fma-mix-insts",
   "HasFmaMixInsts",
   "true",
   "Has v_fma_mix_f32, v_fma_mixlo_f16, v_fma_mixhi_f16 instructions"
 >;
 
+def FeatureDoesNotSupportXNACK : SubtargetFeature<"no-xnack-support",
+  "DoesNotSupportXNACK",
+  "true",
+  "Hardware does not support XNACK"
+>;
+
 // XNACK is disabled if SH_MEM_CONFIG.ADDRESS_MODE = GPUVM on chips that support
 // XNACK. The current default kernel driver setting is:
 // - graphics ring: XNACK disabled
 // - compute ring: XNACK enabled
 //
 // If XNACK is enabled, the VMEM latency can be worse.
 // If XNACK is disabled, the 2 SGPRs can be used for general purposes.
 def FeatureXNACK : SubtargetFeature<"xnack",
class GCNSubtargetFeatureGeneration <string Value,
                                      list<SubtargetFeature> Implies> :
         SubtargetFeatureGeneration <Value, FeatureName, "GCNSubtarget", Implies>;
 
 def FeatureSouthernIslands : GCNSubtargetFeatureGeneration<"SOUTHERN_ISLANDS",
     "southern-islands",
   [FeatureFP64, FeatureLocalMemorySize32768, FeatureMIMG_R128,
   FeatureWavefrontSize64,
   FeatureLDSBankCount32, FeatureMovrel, FeatureTrigReducedRange,
-  FeatureDoesNotSupportSRAMECC]
+  FeatureDoesNotSupportSRAMECC, FeatureDoesNotSupportXNACK]
 >;
 
 def FeatureSeaIslands : GCNSubtargetFeatureGeneration<"SEA_ISLANDS",
     "sea-islands",
   [FeatureFP64, FeatureLocalMemorySize65536, FeatureMIMG_R128,
   FeatureWavefrontSize64, FeatureFlatAddressSpace,
   FeatureCIInsts, FeatureMovrel, FeatureTrigReducedRange,
   FeatureGFX7GFX8GFX9Insts, FeatureDoesNotSupportSRAMECC]

 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_9 : FeatureSet<
   [FeatureGFX9,
    FeatureMadMixInsts,
    FeatureLDSBankCount32,
    FeatureXNACK,
    FeatureCodeObjectV3]>;
!listconcat(FeatureGroup.GFX10_1_Bugs,
      FeatureLDSBankCount32,
      FeatureDLInsts,
      FeatureNSAEncoding,
      FeatureWavefrontSize64,
      FeatureScalarStores,
      FeatureScalarAtomics,
      FeatureScalarFlatScratchInsts,
      FeatureLdsMisalignedBug,
+     FeatureDoesNotSupportXNACK,
      FeatureCodeObjectV3])>;
 
 //===----------------------------------------------------------------------===//
 
 def AMDGPUInstrInfo : InstrInfo {
   let guessInstructionProperties = 1;
   let noNamedPositionallyEncodedOperands = 1;
 }

lib/Target/AMDGPU/AMDGPUSubtarget.h

protected:
   bool FP64FP16Denormals;
   bool FlatForGlobal;
   bool AutoWaitcntBeforeBarrier;
   bool CodeObjectV3;
   bool UnalignedScratchAccess;
   bool UnalignedBufferAccess;
   bool HasApertureRegs;
   bool EnableXNACK;
+  bool DoesNotSupportXNACK;
   bool EnableCuMode;
   bool TrapHandler;
 
   // Used as options.
   bool EnableHugePrivateBuffer;
   bool EnableLoadStoreOpt;
   bool EnableUnsafeDSOffsetFolding;
   bool EnableSIScheduler;

lib/Target/AMDGPU/AMDGPUSubtarget.cpp

GCNSubtarget::initializeSubtargetDependencies(const Triple &TT,
   // 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,+sram-ecc,");
+  SmallString<256> FullFS("+promote-alloca,+load-store-opt,+sram-ecc,+xnack,");
 
   if (isAmdHsaOS()) // Turn on FlatForGlobal for HSA.
     FullFS += "+flat-for-global,+unaligned-buffer-access,+trap-handler,";
 
   // FIXME: I don't think think Evergreen has any useful support for
   // denormals, but should be checked. Should we issue a warning somewhere
   // if someone tries to enable these?
   if (getGeneration() >= AMDGPUSubtarget::SOUTHERN_ISLANDS) {
GCNSubtarget::initializeSubtargetDependencies(const Triple &TT,
   }
 
   // Don't crash on invalid devices.
   if (WavefrontSize == 0)
     WavefrontSize = 64;
 
   HasFminFmaxLegacy = getGeneration() < AMDGPUSubtarget::VOLCANIC_ISLANDS;
 
+  if (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;
   }
 
GCNSubtarget::GCNSubtarget(const Triple &TT, StringRef GPU, StringRef FS,
     FlatForGlobal(false),
     AutoWaitcntBeforeBarrier(false),
     CodeObjectV3(false),
     UnalignedScratchAccess(false),
     UnalignedBufferAccess(false),
 
     HasApertureRegs(false),
     EnableXNACK(false),
+    DoesNotSupportXNACK(false),
     EnableCuMode(false),
     TrapHandler(false),
 
     EnableHugePrivateBuffer(false),
     EnableLoadStoreOpt(false),
     EnableUnsafeDSOffsetFolding(false),
     EnableSIScheduler(false),
     EnableDS128(false),

test/CodeGen/AMDGPU/break-vmem-soft-clauses.mir

 # RUN: llc -march=amdgcn -mcpu=carrizo -verify-machineinstrs -run-pass post-RA-hazard-rec %s -o - | FileCheck -check-prefixes=GCN,XNACK %s
+
+# Make sure the default assumption is xnack enabled with no cpu
+# RUN: llc -march=amdgcn  -verify-machineinstrs -mattr=+volcanic-islands -run-pass post-RA-hazard-rec %s -o - | FileCheck -check-prefixes=GCN,XNACK %s
 # RUN: llc -march=amdgcn -mcpu=fiji -verify-machineinstrs -run-pass post-RA-hazard-rec %s -o - | FileCheck -check-prefixes=GCN,NOXNACK %s
 ---
 # Trivial clause at beginning of program
 name: trivial_clause_load_flat4_x1
 
 body: |
   bb.0:
     ; GCN-LABEL: name: trivial_clause_load_flat4_x1

test/CodeGen/AMDGPU/spill-cfg-position.ll

-; RUN:  llc -amdgpu-scalarize-global-loads=false  -march=amdgcn -verify-machineinstrs -stress-regalloc=6 < %s | FileCheck %s
+; RUN: llc -amdgpu-scalarize-global-loads=false -march=amdgcn -mcpu=tahiti -verify-machineinstrs -stress-regalloc=6 < %s | FileCheck %s
 
 ; Inline spiller can decide to move a spill as early as possible in the basic block.
 ; It will skip phis and label, but we also need to make sure it skips instructions
 ; in the basic block prologue which restore exec mask.
 ; Make sure instruction to restore exec mask immediately follows label
 
 ; CHECK-LABEL: {{^}}spill_cfg_position:
 ; CHECK: s_cbranch_execz [[LABEL1:BB[0-9_]+]]

test/Transforms/Inline/AMDGPU/inline-target-feature-xnack.ll

+; RUN: opt -mtriple=amdgcn-amd-amdhsa -S -inline < %s | FileCheck %s
+; RUN: opt -mtriple=amdgcn-amd-amdhsa -S -passes='cgscc(inline)' < %s | FileCheck %s
+
+define i32 @func_default() #0 {
+  ret i32 0
+}
+
+define i32 @func_xnack_enabled() #1 {
+  ret i32 0
+}
+
+define i32 @func_xnack_disabled() #2 {
+  ret i32 0
+}
+
+; CHECK-LABEL: @default_call_default(
+; CHECK-NEXT: ret i32 0
+define i32 @default_call_default() #0 {
+  %call = call i32 @func_default()
+  ret i32 %call
+}
+
+; CHECK-LABEL: @xnack_enabled_call_default(
+; CHECK-NEXT: ret i32 0
+define i32 @xnack_enabled_call_default() #1 {
+  %call = call i32 @func_default()
+  ret i32 %call
+}
+
+; CHECK-LABEL: @xnack_enabled_call_xnack_enabled(
+; CHECK-NEXT: ret i32 0
+define i32 @xnack_enabled_call_xnack_enabled() #1 {
+  %call = call i32 @func_xnack_enabled()
+  ret i32 %call
+}
+
+; CHECK-LABEL: @xnack_enabled_call_xnack_disabled(
+; CHECK-NEXT: ret i32 0
+define i32 @xnack_enabled_call_xnack_disabled() #1 {
+  %call = call i32 @func_xnack_disabled()
+  ret i32 %call
+}
+
+; CHECK-LABEL: @xnack_disabled_call_default(
+; CHECK-NEXT: ret i32 0
+define i32 @xnack_disabled_call_default() #2 {
+  %call = call i32 @func_default()
+  ret i32 %call
+}
+
+; CHECK-LABEL: @xnack_disabled_call_xnack_enabled(
+; CHECK-NEXT: ret i32 0
+define i32 @xnack_disabled_call_xnack_enabled() #2 {
+  %call = call i32 @func_xnack_enabled()
+  ret i32 %call
+}
+
+; CHECK-LABEL: @xnack_disabled_call_xnack_disabled(
+; CHECK-NEXT: ret i32 0
+define i32 @xnack_disabled_call_xnack_disabled() #2 {
+  %call = call i32 @func_xnack_disabled()
+  ret i32 %call
+}
+
+attributes #0 = { nounwind }
+attributes #1 = { nounwind "target-features"="+xnack" }
+attributes #2 = { nounwind "target-features"="-xnack" }