[PATCH] D76887: AMDGPU: Make HIPToolChain a subclass of ROCMToolChain

[Matt Arsenault via Phabricator via cfe-commits]

arsenm created this revision.
arsenm added reviewers: yaxunl, gregrodgers.
Herald added subscribers: t-tye, tpr, dstuttard, wdng, kzhuravl.
arsenm added parent revisions: D59321: WIP: AMDGPU: Teach toolchain to link 
rocm device libs, D76862: HIP: Ensure new denormal mode attributes are set.

This fixes some code duplication. This is also a step towards
consolidating builtin library handling.


https://reviews.llvm.org/D76887

Files:
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/lib/Driver/ToolChains/HIP.h


Index: clang/lib/Driver/ToolChains/HIP.h
===
--- clang/lib/Driver/ToolChains/HIP.h
+++ clang/lib/Driver/ToolChains/HIP.h
@@ -11,6 +11,7 @@
 
 #include "clang/Driver/ToolChain.h"
 #include "clang/Driver/Tool.h"
+#include "AMDGPU.h"
 
 namespace clang {
 namespace driver {
@@ -72,7 +73,7 @@
 
 namespace toolchains {
 
-class LLVM_LIBRARY_VISIBILITY HIPToolChain : public ToolChain {
+class LLVM_LIBRARY_VISIBILITY HIPToolChain final : public ROCMToolChain {
 public:
   HIPToolChain(const Driver &D, const llvm::Triple &Triple,
 const ToolChain &HostTC, const llvm::opt::ArgList &Args);
@@ -115,11 +116,6 @@
 
   unsigned GetDefaultDwarfVersion() const override { return 4; }
 
-  llvm::DenormalMode getDefaultDenormalModeForType(
-const llvm::opt::ArgList &DriverArgs,
-Action::OffloadKind DeviceOffloadKind,
-const llvm::fltSemantics *FPType = nullptr) const override;
-
   const ToolChain &HostTC;
 
 protected:
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -268,40 +268,12 @@
 
 HIPToolChain::HIPToolChain(const Driver &D, const llvm::Triple &Triple,
  const ToolChain &HostTC, const ArgList &Args)
-: ToolChain(D, Triple, Args), HostTC(HostTC) {
+: ROCMToolChain(D, Triple, Args), HostTC(HostTC) {
   // Lookup binaries into the driver directory, this is used to
   // discover the clang-offload-bundler executable.
   getProgramPaths().push_back(getDriver().Dir);
 }
 
-// FIXME: Duplicated in AMDGPUToolChain
-llvm::DenormalMode HIPToolChain::getDefaultDenormalModeForType(
-const llvm::opt::ArgList &DriverArgs, Action::OffloadKind 
DeviceOffloadKind,
-const llvm::fltSemantics *FPType) const {
-  // Denormals should always be enabled for f16 and f64.
-  if (!FPType || FPType != &llvm::APFloat::IEEEsingle())
-return llvm::DenormalMode::getIEEE();
-
-  if (DeviceOffloadKind == Action::OFK_Cuda) {
-if (FPType && FPType == &llvm::APFloat::IEEEsingle() &&
-DriverArgs.hasFlag(options::OPT_fcuda_flush_denormals_to_zero,
-   options::OPT_fno_cuda_flush_denormals_to_zero,
-   false))
-  return llvm::DenormalMode::getPreserveSign();
-  }
-
-  const StringRef GpuArch = DriverArgs.getLastArgValue(options::OPT_mcpu_EQ);
-  auto Kind = llvm::AMDGPU::parseArchAMDGCN(GpuArch);
-
-  // TODO: There are way too many flags that change this. Do we need to check
-  // them all?
-  bool DAZ = DriverArgs.hasArg(options::OPT_cl_denorms_are_zero) ||
-!AMDGPUToolChain::getDefaultDenormsAreZeroForTarget(Kind);
-  // Outputs are flushed to zero, preserving sign
-  return DAZ ? llvm::DenormalMode::getPreserveSign() :
-   llvm::DenormalMode::getIEEE();
-}
-
 void HIPToolChain::addClangTargetOptions(
 const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args,


Index: clang/lib/Driver/ToolChains/HIP.h
===
--- clang/lib/Driver/ToolChains/HIP.h
+++ clang/lib/Driver/ToolChains/HIP.h
@@ -11,6 +11,7 @@
 
 #include "clang/Driver/ToolChain.h"
 #include "clang/Driver/Tool.h"
+#include "AMDGPU.h"
 
 namespace clang {
 namespace driver {
@@ -72,7 +73,7 @@
 
 namespace toolchains {
 
-class LLVM_LIBRARY_VISIBILITY HIPToolChain : public ToolChain {
+class LLVM_LIBRARY_VISIBILITY HIPToolChain final : public ROCMToolChain {
 public:
   HIPToolChain(const Driver &D, const llvm::Triple &Triple,
 const ToolChain &HostTC, const llvm::opt::ArgList &Args);
@@ -115,11 +116,6 @@
 
   unsigned GetDefaultDwarfVersion() const override { return 4; }
 
-  llvm::DenormalMode getDefaultDenormalModeForType(
-const llvm::opt::ArgList &DriverArgs,
-Action::OffloadKind DeviceOffloadKind,
-const llvm::fltSemantics *FPType = nullptr) const override;
-
   const ToolChain &HostTC;
 
 protected:
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -268,40 +268,12 @@
 
 HIPToolChain::HIPToolChain(const Driver &D, const llvm::Triple &Triple,
  const ToolChain &HostTC, const ArgList &Args)
-: ToolChain(D, Triple, Args), HostTC(HostTC) {
+: ROCM

[PATCH] D76887: AMDGPU: Make HIPToolChain a subclass of ROCMToolChain

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/lib/Driver/ToolChains/HIP.cpp:271
  const ToolChain &HostTC, const ArgList &Args)
-: ToolChain(D, Triple, Args), HostTC(HostTC) {
+: ROCMToolChain(D, Triple, Args), HostTC(HostTC) {
   // Lookup binaries into the driver directory, this is used to

I did not see ROCMToolChain in clang trunk. Is it in another patch?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76887/new/

https://reviews.llvm.org/D76887



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76887: AMDGPU: Make HIPToolChain a subclass of ROCMToolChain

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76887/new/

https://reviews.llvm.org/D76887



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76887: AMDGPU: Make HIPToolChain a subclass of ROCMToolChain

[Matt Arsenault via Phabricator via cfe-commits]

arsenm closed this revision.
arsenm added a comment.

175e42303bb2a4253c6512b1ae05b32b0004 
, rebased 
to avoid dependence on ROCM toolchain patch


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76887/new/

https://reviews.llvm.org/D76887



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits