AMDGPU: Fix target options fp32/64-denormals

[yaxunl]

Summary

Fix target options for fp32/64-denormals so that

+fp64-denormals is set if fp64 is supported
-fp32-denormals if fp32 denormals is not supported, or -cl-denorms-are-zero is set
+fp32-denormals if fp32 denormals is supported and -cl-denorms-are-zero is not set

If target feature fp32/64-denormals is explicitly set, they will override default options and options deduced from -cl-denorms-are-zero.

[• tstellarAMD]

Once small comment otherwise. LGTM.

lib/Basic/Targets.cpp

1962 ↗

(On Diff #71183)

I think this name is a little confusing, because the hardware does support fp32 denorms. I would change this to something like hasFullSpeedFP32Denorms.

[yaxunl]

lib/Basic/Targets.cpp

1962 ↗

(On Diff #71183)

I will change that when committing. Thanks.

cfe/trunk/lib/Basic/Targets.cpp

enum GPUKind {
     GK_GFX6,
     GK_GFX7,
     GK_GFX8
   } GPU;
 
   bool hasFP64:1;
   bool hasFMAF:1;
   bool hasLDEXPF:1;
-  bool hasDenormSupport:1;
+  bool hasFullSpeedFP32Denorms:1;
 
   static bool isAMDGCN(const llvm::Triple &TT) {
     return TT.getArch() == llvm::Triple::amdgcn;
   }
 
 public:
   AMDGPUTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
     : TargetInfo(Triple) ,
       GPU(isAMDGCN(Triple) ? GK_GFX6 : GK_R600),
       hasFP64(false),
       hasFMAF(false),
       hasLDEXPF(false),
-      hasDenormSupport(false){
+      hasFullSpeedFP32Denorms(false){
     if (getTriple().getArch() == llvm::Triple::amdgcn) {
       hasFP64 = true;
       hasFMAF = true;
       hasLDEXPF = true;
     }
-    if (Opts.CPU == "fiji")
-      hasDenormSupport = true;
 
     resetDataLayout(getTriple().getArch() == llvm::Triple::amdgcn ?
                     DataLayoutStringSI : DataLayoutStringR600);
 
     AddrSpaceMap = &AMDGPUAddrSpaceMap;
     UseAddrSpaceMapMangling = true;
   }
 
public:
   }
 
   bool initFeatureMap(llvm::StringMap<bool> &Features,
                       DiagnosticsEngine &Diags, StringRef CPU,
                       const std::vector<std::string> &FeatureVec) const override;
 
   void adjustTargetOptions(const CodeGenOptions &CGOpts,
                            TargetOptions &TargetOpts) const override {
-    if (!hasDenormSupport)
-      return;
     bool hasFP32Denormals = false;
     bool hasFP64Denormals = false;
     for (auto &I : TargetOpts.FeaturesAsWritten) {
       if (I == "+fp32-denormals" || I == "-fp32-denormals")
         hasFP32Denormals = true;
       if (I == "+fp64-denormals" || I == "-fp64-denormals")
         hasFP64Denormals = true;
     }
     if (!hasFP32Denormals)
-      TargetOpts.Features.push_back((Twine(CGOpts.FlushDenorm ? '-' : '+') +
-                                     Twine("fp32-denormals")).str());
+      TargetOpts.Features.push_back((Twine(hasFullSpeedFP32Denorms &&
+          !CGOpts.FlushDenorm ? '+' : '-') + Twine("fp32-denormals")).str());
+    // Always do not flush fp64 denorms.
     if (!hasFP64Denormals && hasFP64)
-      TargetOpts.Features.push_back((Twine(CGOpts.FlushDenorm ? '-' : '+') +
-                                     Twine("fp64-denormals")).str());
+      TargetOpts.Features.push_back("+fp64-denormals");
   }
 
   ArrayRef<Builtin::Info> getTargetBuiltins() const override {
     return llvm::makeArrayRef(BuiltinInfo,
                         clang::AMDGPU::LastTSBuiltin - Builtin::FirstTSBuiltin);
   }
 
   void getTargetDefines(const LangOptions &Opts,

cfe/trunk/test/CodeGenOpenCL/denorms-are-zero.cl

 // RUN: %clang_cc1 -S -cl-denorms-are-zero -o - %s 2>&1
 // RUN: %clang_cc1 -emit-llvm -cl-denorms-are-zero -o - -triple amdgcn--amdhsa -target-cpu fiji %s | FileCheck %s
 // RUN: %clang_cc1 -emit-llvm -o - -triple amdgcn--amdhsa -target-cpu fiji %s | FileCheck %s --check-prefix=CHECK-DENORM
+// RUN: %clang_cc1 -emit-llvm -target-feature +fp32-denormals -target-feature -fp64-denormals -cl-denorms-are-zero -o - -triple amdgcn--amdhsa -target-cpu fiji %s | FileCheck --check-prefix=CHECK-FEATURE %s
 
 // For non-amdgcn targets, this test just checks that the -cl-denorms-are-zero argument is accepted
 // by clang.  This option is currently a no-op, which is allowed by the
 // OpenCL specification.
 
+// For amdgcn target cpu fiji, fp32 should be flushed since fiji does not support fp32 denormals, unless +fp32-denormals is
+// explicitly set. amdgcn target always do not flush fp64 denormals.
+
 // CHECK-DENORM-LABEL: define void @f()
-// CHECK-DENORM: attributes #{{[0-9]*}} = {{{[^}]*}} "target-features"="{{[^"]*}}+fp32-denormals,+fp64-denormals{{[^"]*}}"
+// CHECK-DENORM: attributes #{{[0-9]*}} = {{{[^}]*}} "target-features"="{{[^"]*}}+fp64-denormals,{{[^"]*}}-fp32-denormals{{[^"]*}}"
 // CHECK-LABEL: define void @f()
-// CHECK-NOT: attributes #{{[0-9]*}} = {{{[^}]*}} "target-features"="{{[^"]*}}+fp32-denormals,+fp64-denormals{{[^"]*}}"
+// CHECK: attributes #{{[0-9]*}} = {{{[^}]*}} "target-features"="{{[^"]*}}+fp64-denormals,{{[^"]*}}-fp32-denormals{{[^"]*}}"
+// CHECK-FEATURE-LABEL: define void @f()
+// CHECK-FEATURE: attributes #{{[0-9]*}} = {{{[^}]*}} "target-features"="{{[^"]*}}+fp32-denormals,{{[^"]*}}-fp64-denormals{{[^"]*}}"
 void f() {}