AMDGPU: Disable fp64 support on pre-GCN asics

[jvesely]

Summary

it's not implemented
Passing +fp64-fp16-denormal feature enables fp64 even on asics that don't support it

[arsenm]

Fp64 denormals are always supported

[jvesely]

In D39931#922738, @arsenm wrote:

Fp64 denormals are always supported

This makes no sense, it was clearly disabled on line 70 before for <= NI. This change just makes sure it does not enable fp64 en masse on all asics in the process.

FP64 denormals are not supported if fp64 is not supported.
Moreover, table 8.1 in R900 ISA docs show that adding a denormal has no effect if the other operand is finite, and adding 2 denormals is 0.
The same for EG in table 9.1 (R800 ISA docs).

[arsenm]

FP64 support without denormals doesn't make sense. They are mandatory in the OpenCL spec

[jvesely]

In D39931#922825, @arsenm wrote:

FP64 support without denormals doesn't make sense. They are mandatory in the OpenCL spec

so what? this has clearly been disabled before, and it stays disabled after this change, just with less damage done in the process..

enabling fp64 denormals can be considered when fp64 support is added to eg/ni targets.

[arsenm]

Fp64 denormals are always supported

In D39931#922846, @jvesely wrote:

In D39931#922825, @arsenm wrote:

FP64 support without denormals doesn't make sense. They are mandatory in the OpenCL spec

so what? this has clearly been disabled before, and it stays disabled after this change, just with less damage done in the process..

enabling fp64 denormals can be considered when fp64 support is added to eg/ni targets.

The Evergreen manual lists double_denorm_flush_input and double_denorm_force_underflow_to_zero modes, so that implies they are supported. I don't care that much because the f64 support is missing, but I'm not sure what problem you are trying to solve here

lib/Target/AMDGPU/AMDGPUSubtarget.cpp

62 ↗

(On Diff #122558)

You shouldn't explicitly disable them either if it's not a recognized feature for r600

[jvesely]

In D39931#922904, @arsenm wrote:

Fp64 denormals are always supported

In D39931#922846, @jvesely wrote:

In D39931#922825, @arsenm wrote:

FP64 support without denormals doesn't make sense. They are mandatory in the OpenCL spec

so what? this has clearly been disabled before, and it stays disabled after this change, just with less damage done in the process..

enabling fp64 denormals can be considered when fp64 support is added to eg/ni targets.

The Evergreen manual lists double_denorm_flush_input and double_denorm_force_underflow_to_zero modes, so that implies they are supported.

I'll double check if it matches it specs when fp64 gets added, afaik it's not consistent for all instructions.

I don't care that much because the f64 support is missing, but I'm not sure what problem you are trying to solve here

the problem is that fp64-fp16-denormals feature depends on fp64 feature, thus passing "+fp64-fp16-denormals" string enables fp64 on all ASICs, including those (like almost all EG) that don't support fp64. THus I can't use 'if (hasFP64())' to in isel.
This needs to work for FMA (not available without fp64, the instruction executes fine but returns 0).

The alternative is to drop fp64-fp16-denormlas dependency on fp64 and change the has*() function to check for both.

[jvesely]

ping

lib/Target/AMDGPU/AMDGPUSubtarget.cpp

62 ↗

(On Diff #122558)

it is recognized, just not supported atm

[jvesely]

just didable fp64 on pregcn asics

[arsenm]

LGTM

llvm/trunk/lib/Target/AMDGPU/AMDGPUSubtarget.h

public:
   bool hasIntClamp() const {
     return HasIntClamp;
   }
 
   bool hasVOP3PInsts() const {
     return HasVOP3PInsts;
   }
 
-  bool hasHWFP64() const {
+  bool hasFP64() const {
     return FP64;
   }
 
   bool hasFastFMAF32() const {
     return FastFMAF32;
   }
 
   bool hasHalfRate64Ops() const {

llvm/trunk/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

AMDGPUSubtarget::initializeSubtargetDependencies(const Triple &TT,
   // On SI+, we want FP64 denormals to be on by default. FP32 denormals can be
   // enabled, but some instructions do not respect them and they run at the
   // double precision rate, so don't enable by default.
   //
   // 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.
 
-  SmallString<256> FullFS("+promote-alloca,+fp64-fp16-denormals,+dx10-clamp,+load-store-opt,");
+  SmallString<256> FullFS("+promote-alloca,+dx10-clamp,+load-store-opt,");
+
   if (isAmdHsaOS()) // Turn on FlatForGlobal for HSA.
     FullFS += "+flat-address-space,+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) {
+    FullFS += "+fp64-fp16-denormals,";
+  } else {
+    FullFS += "-fp32-denormals,";
+  }
+
   FullFS += FS;
 
   ParseSubtargetFeatures(GPU, FullFS);
 
+  // We don't support FP64 for EG/NI atm.
+  assert(!hasFP64() || (getGeneration() >= AMDGPUSubtarget::SOUTHERN_ISLANDS));
+
   // Unless +-flat-for-global is specified, turn on FlatForGlobal for all OS-es
   // on VI and newer hardware to avoid assertion failures due to missing ADDR64
   // variants of MUBUF instructions.
   if (!hasAddr64() && !FS.contains("flat-for-global")) {
     FlatForGlobal = true;
   }
 
-  // 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::NORTHERN_ISLANDS) {
-    FP64FP16Denormals = false;
-    FP32Denormals = false;
-  }
-
   // Set defaults if needed.
   if (MaxPrivateElementSize == 0)
     MaxPrivateElementSize = 4;
 
   if (LDSBankCount == 0)
     LDSBankCount = 32;
 
   if (TT.getArch() == Triple::amdgcn) {

llvm/trunk/lib/Target/AMDGPU/R600Processors.td

def : Processor<"r630", R600_VLIW5_Itin,
   [FeatureR600, FeatureWavefrontSize32, FeatureVertexCache]
 >;
 
 def : Processor<"rs880", R600_VLIW5_Itin,
   [FeatureR600, FeatureWavefrontSize16]
 >;
 
 def : Processor<"rv670", R600_VLIW5_Itin,
-  [FeatureR600, FeatureWavefrontSize64, FeatureVertexCache, FeatureFP64]
+  [FeatureR600, FeatureWavefrontSize64, FeatureVertexCache]
 >;
 
 //===----------------------------------------------------------------------===//
 // Radeon HD 4000 Series (R700).
 //===----------------------------------------------------------------------===//
 
 def : Processor<"rv710", R600_VLIW5_Itin,
   [FeatureR700, FeatureWavefrontSize32, FeatureVertexCache]
 >;
 
 def : Processor<"rv730", R600_VLIW5_Itin,
   [FeatureR700, FeatureWavefrontSize32, FeatureVertexCache]
 >;
 
 def : Processor<"rv770", R600_VLIW5_Itin,
-  [FeatureR700, FeatureWavefrontSize64, FeatureVertexCache, FeatureFP64]
+  [FeatureR700, FeatureWavefrontSize64, FeatureVertexCache]
 >;
 
 //===----------------------------------------------------------------------===//
 // Radeon HD 5000 Series (Evergreen).
 //===----------------------------------------------------------------------===//
 
 def : Processor<"cedar", R600_VLIW5_Itin,
   [FeatureEvergreen, FeatureWavefrontSize32, FeatureVertexCache,
    FeatureCFALUBug]
 >;
 
 def : Processor<"cypress", R600_VLIW5_Itin,
-  [FeatureEvergreen, FeatureWavefrontSize64, FeatureVertexCache, FeatureFP64]
+  [FeatureEvergreen, FeatureWavefrontSize64, FeatureVertexCache]
 >;
 
 def : Processor<"juniper", R600_VLIW5_Itin,
   [FeatureEvergreen, FeatureWavefrontSize64, FeatureVertexCache]
 >;
 
 def : Processor<"redwood", R600_VLIW5_Itin,
   [FeatureEvergreen, FeatureWavefrontSize64, FeatureVertexCache,
def : Processor<"barts", R600_VLIW5_Itin,
   [FeatureNorthernIslands, FeatureVertexCache, FeatureCFALUBug]
 >;
 
 def : Processor<"caicos", R600_VLIW5_Itin,
   [FeatureNorthernIslands, FeatureCFALUBug]
 >;
 
 def : Processor<"cayman", R600_VLIW4_Itin,
-  [FeatureNorthernIslands, FeatureFP64, FeatureCaymanISA]
+  [FeatureNorthernIslands, FeatureCaymanISA]
 >;
 
 def : Processor<"turks", R600_VLIW5_Itin,
   [FeatureNorthernIslands, FeatureVertexCache, FeatureCFALUBug]
 >;

llvm/trunk/test/CodeGen/AMDGPU/fma.ll

 ; RUN:  llc -amdgpu-scalarize-global-loads=false  -march=amdgcn -mcpu=tahiti -verify-machineinstrs < %s | FileCheck -check-prefix=SI -check-prefix=FUNC %s
-; RUN:  llc -amdgpu-scalarize-global-loads=false  -march=r600 -mcpu=cypress -verify-machineinstrs < %s | FileCheck -check-prefix=EG -check-prefix=FUNC %s
+; XUN:  llc -amdgpu-scalarize-global-loads=false  -march=r600 -mcpu=cypress -verify-machineinstrs < %s | FileCheck -check-prefix=EG -check-prefix=FUNC %s
 
 declare float @llvm.fma.f32(float, float, float) nounwind readnone
 declare <2 x float> @llvm.fma.v2f32(<2 x float>, <2 x float>, <2 x float>) nounwind readnone
 declare <4 x float> @llvm.fma.v4f32(<4 x float>, <4 x float>, <4 x float>) nounwind readnone
 
 declare i32 @llvm.r600.read.tidig.x() nounwind readnone
 
 ; FUNC-LABEL: {{^}}fma_f32: