Skip to content

Commit

Permalink
[HIP] emit macro __HIP_NO_IMAGE_SUPPORT
Browse files Browse the repository at this point in the history
HIP texture/image support is optional as some devices
do not have image instructions. A macro __HIP_NO_IMAGE_SUPPORT
is defined for device not supporting images (https://github.com/ROCm-Developer-Tools/HIP/blob/d0448aa4c4dd0f4b29ccf6a663b7f5ad9f5183e0/docs/reference/kernel_language.md?plain=1#L426 )

Currently the macro is defined by HIP header based on predefined macros
for GPU, e.g __gfx*__ , which is error prone. This patch let clang
emit the predefined macro.

Reviewed by: Matt Arsenault, Artem Belevich

Differential Revision: https://reviews.llvm.org/D151349
  • Loading branch information
yxsamliu committed Jun 15, 2023
1 parent dc895d0 commit c0f0d50
Show file tree
Hide file tree
Showing 9 changed files with 79 additions and 2 deletions.
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticFrontendKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ def warn_fe_backend_unsupported_fp_exceptions : Warning<
def warn_fe_backend_invalid_feature_flag : Warning<
"feature flag '%0' must start with either '+' to enable the feature or '-'"
" to disable it; flag ignored">, InGroup<InvalidCommandLineArgument>;
def warn_fe_backend_readonly_feature_flag : Warning<
"feature flag '%0' is ignored since the feature is read only">,
InGroup<InvalidCommandLineArgument>;

def err_incompatible_fp_eval_method_options : Error<
"option 'ffp-eval-method' cannot be used with option "
Expand Down
15 changes: 15 additions & 0 deletions clang/include/clang/Basic/TargetInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/Frontend/OpenMP/OMPGridValues.h"
#include "llvm/IR/DerivedTypes.h"
#include "llvm/Support/DataTypes.h"
Expand Down Expand Up @@ -267,6 +268,12 @@ class TargetInfo : public TransferrableTargetInfo,
// as a DataLayout object.
void resetDataLayout(StringRef DL, const char *UserLabelPrefix = "");

// Target features that are read-only and should not be disabled/enabled
// by command line options. Such features are for emitting predefined
// macros or checking availability of builtin functions and can be omitted
// in function attributes in IR.
llvm::StringSet<> ReadOnlyFeatures;

public:
/// Construct a target for the given options.
///
Expand Down Expand Up @@ -1394,6 +1401,11 @@ class TargetInfo : public TransferrableTargetInfo,
return false;
}

/// Determine whether the given target feature is read only.
bool isReadOnlyFeature(StringRef Feature) const {
return ReadOnlyFeatures.count(Feature);
}

/// Identify whether this target supports multiversioning of functions,
/// which requires support for cpu_supports and cpu_is functionality.
bool supportsMultiVersioning() const {
Expand Down Expand Up @@ -1711,6 +1723,9 @@ class TargetInfo : public TransferrableTargetInfo,
: std::optional<VersionTuple>();
}

/// Whether to support HIP image/texture API's.
virtual bool hasHIPImageSupport() const { return true; }

protected:
/// Copy type and layout related info.
void copyAuxTarget(const TargetInfo *Aux);
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/Basic/TargetInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,8 @@ bool TargetInfo::initFeatureMap(
// Apply the feature via the target.
if (Name[0] != '+' && Name[0] != '-')
Diags.Report(diag::warn_fe_backend_invalid_feature_flag) << Name;
else if (isReadOnlyFeature(Name.substr(1)))
Diags.Report(diag::warn_fe_backend_readonly_feature_flag) << Name;
else
setFeatureEnabled(Features, Name.substr(1), Name[0] == '+');
}
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Basic/Targets/AMDGPU.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ AMDGPUTargetInfo::AMDGPUTargetInfo(const llvm::Triple &Triple,

MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
CUMode = !(GPUFeatures & llvm::AMDGPU::FEATURE_WGP);
ReadOnlyFeatures.insert("image-insts");
}

void AMDGPUTargetInfo::adjust(DiagnosticsEngine &Diags, LangOptions &Opts) {
Expand Down
7 changes: 7 additions & 0 deletions clang/lib/Basic/Targets/AMDGPU.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : public TargetInfo {
/// Whether to use cumode or WGP mode. True for cumode. False for WGP mode.
bool CUMode;

/// Whether having image instructions.
bool HasImage = false;

/// Target ID is device name followed by optional feature name postfixed
/// by plus or minus sign delimitted by colon, e.g. gfx908:xnack+:sramecc-.
/// If the target ID contains feature+, map it to true.
Expand Down Expand Up @@ -449,6 +452,8 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : public TargetInfo {
CUMode = true;
else if (F == "-cumode")
CUMode = false;
else if (F == "+image-insts")
HasImage = true;
bool IsOn = F.front() == '+';
StringRef Name = StringRef(F).drop_front();
if (!llvm::is_contained(TargetIDFeatures, Name))
Expand All @@ -469,6 +474,8 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : public TargetInfo {
return getCanonicalTargetID(getArchNameAMDGCN(GPUKind),
OffloadArchFeatures);
}

bool hasHIPImageSupport() const override { return HasImage; }
};

} // namespace targets
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/CodeGen/CodeGenModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2299,6 +2299,9 @@ bool CodeGenModule::GetCPUAndFeaturesAttributes(GlobalDecl GD,
AddedAttr = true;
}
if (!Features.empty() && SetTargetFeatures) {
llvm::erase_if(Features, [&](const std::string& F) {
return getTarget().isReadOnlyFeature(F.substr(1));
});
llvm::sort(Features);
Attrs.addAttribute("target-features", llvm::join(Features, ","));
AddedAttr = true;
Expand Down
5 changes: 4 additions & 1 deletion clang/lib/Frontend/InitPreprocessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -583,8 +583,11 @@ static void InitializeStandardPredefinedMacros(const TargetInfo &TI,
Builder.defineMacro("__HIP_MEMORY_SCOPE_WORKGROUP", "3");
Builder.defineMacro("__HIP_MEMORY_SCOPE_AGENT", "4");
Builder.defineMacro("__HIP_MEMORY_SCOPE_SYSTEM", "5");
if (LangOpts.CUDAIsDevice)
if (LangOpts.CUDAIsDevice) {
Builder.defineMacro("__HIP_DEVICE_COMPILE__");
if (!TI.hasHIPImageSupport())
Builder.defineMacro("__HIP_NO_IMAGE_SUPPORT", "1");
}
if (LangOpts.GPUDefaultStream ==
LangOptions::GPUDefaultStreamKind::PerThread)
Builder.defineMacro("HIP_API_PER_THREAD_DEFAULT_STREAM");
Expand Down
19 changes: 19 additions & 0 deletions clang/test/Driver/hip-macros.hip
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,22 @@
// WARN-CUMODE-NOT: warning: ignoring '-mno-cumode' option as it is not currently supported for processor 'gfx906' [-Woption-ignored]
// CUMODE-ON-DAG: #define __AMDGCN_CUMODE__ 1
// CUMODE-OFF-DAG: #define __AMDGCN_CUMODE__ 0

// RUN: %clang -E -dM --offload-arch=gfx90a --cuda-device-only -nogpuinc -nogpulib \
// RUN: %s 2>&1 | FileCheck --check-prefixes=IMAGE,NOWARN %s
// RUN: %clang -E -dM --offload-arch=gfx1100 --cuda-device-only -nogpuinc -nogpulib \
// RUN: %s 2>&1 | FileCheck --check-prefixes=IMAGE,NOWARN %s
// RUN: %clang -E -dM --offload-arch=gfx940 --cuda-device-only -nogpuinc -nogpulib \
// RUN: %s 2>&1 | FileCheck --check-prefixes=NOIMAGE,NOWARN %s
// RUN: %clang -E -dM --offload-arch=gfx941 --cuda-device-only -nogpuinc -nogpulib \
// RUN: %s 2>&1 | FileCheck --check-prefixes=NOIMAGE,NOWARN %s
// RUN: %clang -E -dM --offload-arch=gfx942 --cuda-device-only -nogpuinc -nogpulib \
// RUN: %s 2>&1 | FileCheck --check-prefixes=NOIMAGE,NOWARN %s
// RUN: %clang -E -dM --offload-arch=gfx1100 --cuda-device-only -nogpuinc -nogpulib \
// RUN: -Xclang -target-feature -Xclang "-image-insts" %s 2>&1 | FileCheck --check-prefixes=IMAGE,WARN %s
// RUN: %clang -E -dM --offload-arch=gfx940 --cuda-device-only -nogpuinc -nogpulib \
// RUN: -Xclang -target-feature -Xclang "+image-insts" %s 2>&1 | FileCheck --check-prefixes=NOIMAGE,WARN %s
// NOWARN-NOT: warning
// WARN: warning: feature flag '{{[+|-]}}image-insts' is ignored since the feature is read only [-Winvalid-command-line-argument]
// IMAGE-NOT: #define __HIP_NO_IMAGE_SUPPORT
// NOIMAGE: #define __HIP_NO_IMAGE_SUPPORT 1
26 changes: 25 additions & 1 deletion llvm/lib/TargetParser/TargetParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ void AMDGPU::fillAMDGPUFeatureMap(StringRef GPU, const Triple &T,
Features["gfx10-3-insts"] = true;
Features["gfx11-insts"] = true;
Features["atomic-fadd-rtn-insts"] = true;
Features["image-insts"] = true;
break;
case GK_GFX1036:
case GK_GFX1035:
Expand All @@ -301,6 +302,7 @@ void AMDGPU::fillAMDGPUFeatureMap(StringRef GPU, const Triple &T,
Features["gfx9-insts"] = true;
Features["gfx10-insts"] = true;
Features["gfx10-3-insts"] = true;
Features["image-insts"] = true;
Features["s-memrealtime"] = true;
Features["s-memtime-inst"] = true;
break;
Expand All @@ -322,6 +324,7 @@ void AMDGPU::fillAMDGPUFeatureMap(StringRef GPU, const Triple &T,
Features["gfx8-insts"] = true;
Features["gfx9-insts"] = true;
Features["gfx10-insts"] = true;
Features["image-insts"] = true;
Features["s-memrealtime"] = true;
Features["s-memtime-inst"] = true;
break;
Expand All @@ -333,7 +336,27 @@ void AMDGPU::fillAMDGPUFeatureMap(StringRef GPU, const Triple &T,
Features["atomic-ds-pk-add-16-insts"] = true;
Features["atomic-flat-pk-add-16-insts"] = true;
Features["atomic-global-pk-add-bf16-inst"] = true;
[[fallthrough]];
Features["gfx90a-insts"] = true;
Features["atomic-buffer-global-pk-add-f16-insts"] = true;
Features["atomic-fadd-rtn-insts"] = true;
Features["dot3-insts"] = true;
Features["dot4-insts"] = true;
Features["dot5-insts"] = true;
Features["dot6-insts"] = true;
Features["mai-insts"] = true;
Features["dl-insts"] = true;
Features["dot1-insts"] = true;
Features["dot2-insts"] = true;
Features["dot7-insts"] = true;
Features["dot10-insts"] = true;
Features["gfx9-insts"] = true;
Features["gfx8-insts"] = true;
Features["16-bit-insts"] = true;
Features["dpp"] = true;
Features["s-memrealtime"] = true;
Features["ci-insts"] = true;
Features["s-memtime-inst"] = true;
break;
case GK_GFX90A:
Features["gfx90a-insts"] = true;
Features["atomic-buffer-global-pk-add-f16-insts"] = true;
Expand Down Expand Up @@ -381,6 +404,7 @@ void AMDGPU::fillAMDGPUFeatureMap(StringRef GPU, const Triple &T,
case GK_GFX602:
case GK_GFX601:
case GK_GFX600:
Features["image-insts"] = true;
Features["s-memtime-inst"] = true;
break;
case GK_NONE:
Expand Down

0 comments on commit c0f0d50

Please sign in to comment.