From 2f573e4963ccff289b2ab9513a559550041a95c6 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Tue, 5 Apr 2022 11:23:52 -0700 Subject: [PATCH 1/4] Add option for validator-version. The information will saved into ModuleFlags with key "dx.valver". --- clang/include/clang/Basic/CodeGenOptions.h | 3 + .../clang/Basic/DiagnosticDriverKinds.td | 2 + clang/include/clang/Driver/Options.td | 6 + clang/lib/CodeGen/CGHLSLRuntime.cpp | 55 +++++++++ clang/lib/CodeGen/CGHLSLRuntime.h | 35 ++++++ clang/lib/CodeGen/CMakeLists.txt | 1 + clang/lib/CodeGen/CodeGenModule.cpp | 12 ++ clang/lib/CodeGen/CodeGenModule.h | 9 ++ clang/lib/Driver/ToolChains/Clang.cpp | 15 +++ clang/lib/Driver/ToolChains/HLSL.cpp | 60 +++++++++- clang/lib/Driver/ToolChains/HLSL.h | 3 + clang/test/CodeGenHLSL/validator_version.hlsl | 10 ++ clang/unittests/Driver/ToolChainTest.cpp | 108 ++++++++++++++++++ 13 files changed, 318 insertions(+), 1 deletion(-) create mode 100644 clang/lib/CodeGen/CGHLSLRuntime.cpp create mode 100644 clang/lib/CodeGen/CGHLSLRuntime.h create mode 100644 clang/test/CodeGenHLSL/validator_version.hlsl diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h index 128ca2f5df3c3..ff901c2f74a69 100644 --- a/clang/include/clang/Basic/CodeGenOptions.h +++ b/clang/include/clang/Basic/CodeGenOptions.h @@ -190,6 +190,9 @@ class CodeGenOptions : public CodeGenOptionsBase { /// debug info. std::string DIBugsReportFilePath; + /// The validator version for dxil. + std::string DxilValidatorVersion; + /// The floating-point denormal mode to use. llvm::DenormalMode FPDenormalMode = llvm::DenormalMode::getIEEE(); diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index 4ff6619db8d5c..f41da15888499 100644 --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -657,4 +657,6 @@ def err_drv_target_variant_invalid : Error< def err_drv_invalid_directx_shader_module : Error< "invalid profile : %0">; +def err_drv_invalid_dxil_validator_version : Error< + "invalid validator version : %0\n%1">; } diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 1baa3a6ebec2f..64f62a5350d36 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -6653,6 +6653,7 @@ def _SLASH_ZW : CLJoined<"ZW">; def dxc_Group : OptionGroup<"">, Flags<[DXCOption]>, HelpText<"dxc compatibility options">; + class DXCJoinedOrSeparate : Option<["/", "-"], name, KIND_JOINED_OR_SEPARATE>, Group, Flags<[DXCOption, NoXarchOption]>; @@ -6664,6 +6665,11 @@ def dxc_help : Option<["/", "-", "--"], "help", KIND_JOINED>, def Fo : DXCJoinedOrSeparate<"Fo">, Alias, HelpText<"Output object file.">; +def dxil_validator_version : Option<["/", "-"], "validator-version", KIND_SEPARATE>, + Group, Flags<[DXCOption, NoXarchOption, CC1Option, HelpHidden]>, + HelpText<"Override validator version for module. Format: ;Default: DXIL.dll version or current internal version.">, + MarshallingInfoString>; + def target_profile : DXCJoinedOrSeparate<"T">, MetaVarName<"">, HelpText<"Set target profile.">, Values<"ps_6_0, ps_6_1, ps_6_2, ps_6_3, ps_6_4, ps_6_5, ps_6_6, ps_6_7," diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp new file mode 100644 index 0000000000000..75373a12993cf --- /dev/null +++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp @@ -0,0 +1,55 @@ +//===----- CGHLSLRuntime.cpp - Interface to HLSL Runtimes -----------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This provides an abstract class for HLSL code generation. Concrete +// subclasses of this implement code generation for specific HLSL +// runtime libraries. +// +//===----------------------------------------------------------------------===// + +#include "CGHLSLRuntime.h" +#include "CodeGenModule.h" +#include "clang/Basic/CodeGenOptions.h" +#include "llvm/IR/Metadata.h" +#include "llvm/IR/Module.h" + +using namespace clang; +using namespace CodeGen; +using namespace llvm; + +namespace { +void addDxilValVersion(StringRef ValVersionStr, llvm::Module &M) { + // The validation of ValVersionStr is done at HLSLToolChain::TranslateArgs. + // Assume ValVersionStr is legal here. + auto VerPair = ValVersionStr.split("."); + llvm::APInt APMajor, APMinor; + + if (VerPair.first.getAsInteger(0, APMajor) || + VerPair.second.getAsInteger(0, APMinor)) { + return; + } + uint64_t Major = APMajor.getLimitedValue(); + uint64_t Minor = APMinor.getLimitedValue(); + auto &Ctx = M.getContext(); + IRBuilder<> B(M.getContext()); + MDNode *Val = MDNode::get(Ctx, {ConstantAsMetadata::get(B.getInt32(Major)), + ConstantAsMetadata::get(B.getInt32(Minor))}); + StringRef DxilValKey = "dx.valver"; + M.addModuleFlag(llvm::Module::ModFlagBehavior::AppendUnique, DxilValKey, Val); +} +} // namespace + +CGHLSLRuntime::CGHLSLRuntime(CodeGenModule &CGM) : CGM(CGM) {} + +CGHLSLRuntime::~CGHLSLRuntime() {} + +void CGHLSLRuntime::finishCodeGen() { + auto &CGOpts = CGM.getCodeGenOpts(); + llvm::Module &M = CGM.getModule(); + addDxilValVersion(CGOpts.DxilValidatorVersion, M); +} \ No newline at end of file diff --git a/clang/lib/CodeGen/CGHLSLRuntime.h b/clang/lib/CodeGen/CGHLSLRuntime.h new file mode 100644 index 0000000000000..43581a36a320f --- /dev/null +++ b/clang/lib/CodeGen/CGHLSLRuntime.h @@ -0,0 +1,35 @@ +//===----- CGHLSLRuntime.h - Interface to HLSL Runtimes -----*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This provides an abstract class for HLSL code generation. Concrete +// subclasses of this implement code generation for specific HLSL +// runtime libraries. +// +//===----------------------------------------------------------------------===// + +#pragma once + +namespace clang { + +namespace CodeGen { + +class CodeGenModule; + +class CGHLSLRuntime { +protected: + CodeGenModule &CGM; + +public: + CGHLSLRuntime(CodeGenModule &CGM); + virtual ~CGHLSLRuntime(); + void finishCodeGen(); + +}; + +} +} diff --git a/clang/lib/CodeGen/CMakeLists.txt b/clang/lib/CodeGen/CMakeLists.txt index c0b486daae3ab..0bb5abcf60455 100644 --- a/clang/lib/CodeGen/CMakeLists.txt +++ b/clang/lib/CodeGen/CMakeLists.txt @@ -51,6 +51,7 @@ add_clang_library(clangCodeGen CGExprConstant.cpp CGExprScalar.cpp CGGPUBuiltin.cpp + CGHLSLRuntime.cpp CGLoopInfo.cpp CGNonTrivialStruct.cpp CGObjC.cpp diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 5536626d0691a..d8c41ea6f4247 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -16,6 +16,7 @@ #include "CGCXXABI.h" #include "CGCall.h" #include "CGDebugInfo.h" +#include "CGHLSLRuntime.h" #include "CGObjCRuntime.h" #include "CGOpenCLRuntime.h" #include "CGOpenMPRuntime.h" @@ -146,6 +147,8 @@ CodeGenModule::CodeGenModule(ASTContext &C, const HeaderSearchOptions &HSO, createOpenMPRuntime(); if (LangOpts.CUDA) createCUDARuntime(); + if (LangOpts.HLSL) + createHLSLRuntime(); // Enable TBAA unless it's suppressed. ThreadSanitizer needs TBAA even at O0. if (LangOpts.Sanitize.has(SanitizerKind::Thread) || @@ -262,6 +265,10 @@ void CodeGenModule::createCUDARuntime() { CUDARuntime.reset(CreateNVCUDARuntime(*this)); } +void CodeGenModule::createHLSLRuntime() { + HLSLRuntime.reset(new CGHLSLRuntime(*this)); +} + void CodeGenModule::addReplacement(StringRef Name, llvm::Constant *C) { Replacements[Name] = C; } @@ -805,6 +812,11 @@ void CodeGenModule::Release() { } } + // HLSL related end of code gen work items. + if (LangOpts.HLSL) { + getHLSLRuntime().finishCodeGen(); + } + if (uint32_t PLevel = Context.getLangOpts().PICLevel) { assert(PLevel < 3 && "Invalid PIC Level"); getModule().setPICLevel(static_cast(PLevel)); diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index a95949109c371..b690cfce411b9 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -85,6 +85,7 @@ class CGObjCRuntime; class CGOpenCLRuntime; class CGOpenMPRuntime; class CGCUDARuntime; +class CGHLSLRuntime; class CoverageMappingModuleGen; class TargetCodeGenInfo; @@ -319,6 +320,7 @@ class CodeGenModule : public CodeGenTypeCache { std::unique_ptr OpenCLRuntime; std::unique_ptr OpenMPRuntime; std::unique_ptr CUDARuntime; + std::unique_ptr HLSLRuntime; std::unique_ptr DebugInfo; std::unique_ptr ObjCData; llvm::MDNode *NoObjCARCExceptionsMetadata = nullptr; @@ -511,6 +513,7 @@ class CodeGenModule : public CodeGenTypeCache { void createOpenCLRuntime(); void createOpenMPRuntime(); void createCUDARuntime(); + void createHLSLRuntime(); bool isTriviallyRecursive(const FunctionDecl *F); bool shouldEmitFunction(GlobalDecl GD); @@ -609,6 +612,12 @@ class CodeGenModule : public CodeGenTypeCache { return *CUDARuntime; } + /// Return a reference to the configured HLSL runtime. + CGHLSLRuntime &getHLSLRuntime() { + assert(HLSLRuntime != nullptr); + return *HLSLRuntime; + } + ObjCEntrypoints &getObjCEntrypoints() const { assert(ObjCData != nullptr); return *ObjCData; diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index cbdb463d09682..ae40761c0d851 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -3454,6 +3454,16 @@ static void RenderOpenCLOptions(const ArgList &Args, ArgStringList &CmdArgs, } } +static void RenderHLSLOptions(const ArgList &Args, ArgStringList &CmdArgs, + types::ID InputType) { + const unsigned ForwardedArguments[] = {options::OPT_dxil_validator_version}; + + for (const auto &Arg : ForwardedArguments) + if (const auto *A = Args.getLastArg(Arg)) { + A->renderAsInput(Args, CmdArgs); + } +} + static void RenderARCMigrateToolOptions(const Driver &D, const ArgList &Args, ArgStringList &CmdArgs) { bool ARCMTEnabled = false; @@ -6232,6 +6242,11 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, // Forward -cl options to -cc1 RenderOpenCLOptions(Args, CmdArgs, InputType); + if (C.getDriver().IsDXCMode()) { + // Forward hlsl options to -cc1 + RenderHLSLOptions(Args, CmdArgs, InputType); + } + if (IsHIP) { if (Args.hasFlag(options::OPT_fhip_new_launch_api, options::OPT_fno_hip_new_launch_api, true)) diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp index e8252558eb57d..7a2791a89af53 100644 --- a/clang/lib/Driver/ToolChains/HLSL.cpp +++ b/clang/lib/Driver/ToolChains/HLSL.cpp @@ -22,7 +22,11 @@ using namespace llvm; namespace { const unsigned OfflineLibMinor = 0xF; -const unsigned MaxShaderModel6Minor = 7; +const unsigned MaxDXILMajor = 1; +const unsigned MaxDXILMinor = 7; +const unsigned MaxShaderModel6Minor = MaxDXILMinor; +// TODO:get default validator version from validator. +const StringRef DefaultValidatorVer = "1.7"; bool isLegalVersion(VersionTuple Version, unsigned Major, unsigned MinMinor, unsigned MaxMinor) { @@ -122,6 +126,30 @@ std::string tryParseProfile(StringRef Profile) { return ""; } +bool isLegalValidatorVersion(StringRef ValVersionStr, std::string &ErrorMsg) { + auto VerPair = ValVersionStr.split("."); + llvm::APInt APMajor, APMinor; + + if (VerPair.first.getAsInteger(0, APMajor) || + VerPair.second.getAsInteger(0, APMinor)) { + ErrorMsg = + "Format of validator version is \".\" (ex:\"1.4\")."; + return false; + } + uint64_t Major = APMajor.getLimitedValue(); + uint64_t Minor = APMinor.getLimitedValue(); + if (Major > MaxDXILMajor || (Major == MaxDXILMajor && Minor > MaxDXILMinor)) { + ErrorMsg = "Validator version must be less than or equal to current " + "internal version."; + return false; + } + if (Major == 0 && Minor != 0) { + ErrorMsg = "If validator major version is 0, minor version must also be 0."; + return false; + } + return true; +} + } // namespace /// DirectX Toolchain @@ -145,3 +173,33 @@ HLSLToolChain::ComputeEffectiveClangTriple(const ArgList &Args, return ToolChain::ComputeEffectiveClangTriple(Args, InputType); } } + +DerivedArgList * +HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch, + Action::OffloadKind DeviceOffloadKind) const { + DerivedArgList *DAL = new DerivedArgList(Args.getBaseArgs()); + + const OptTable &Opts = getDriver().getOpts(); + + for (Arg *A : Args) { + if (A->getOption().getID() == options::OPT_dxil_validator_version) { + StringRef ValVerStr = A->getValue(); + std::string ErrorMsg; + if (!isLegalValidatorVersion(ValVerStr, ErrorMsg)) { + getDriver().Diag(diag::err_drv_invalid_dxil_validator_version) + << ValVerStr << ErrorMsg; + + continue; + } + } + DAL->append(A); + } + // Add default validator version if not set. + // TODO: remove this once read validator version from validator. + if (!DAL->hasArg(options::OPT_dxil_validator_version)) { + DAL->AddSeparateArg(nullptr, + Opts.getOption(options::OPT_dxil_validator_version), + DefaultValidatorVer); + } + return DAL; +} diff --git a/clang/lib/Driver/ToolChains/HLSL.h b/clang/lib/Driver/ToolChains/HLSL.h index 2fd9809efd042..a6bc39c1ee9ed 100644 --- a/clang/lib/Driver/ToolChains/HLSL.h +++ b/clang/lib/Driver/ToolChains/HLSL.h @@ -25,6 +25,9 @@ class LLVM_LIBRARY_VISIBILITY HLSLToolChain : public ToolChain { } bool isPICDefaultForced() const override { return false; } + llvm::opt::DerivedArgList * + TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch, + Action::OffloadKind DeviceOffloadKind) const override; std::string ComputeEffectiveClangTriple(const llvm::opt::ArgList &Args, types::ID InputType) const override; }; diff --git a/clang/test/CodeGenHLSL/validator_version.hlsl b/clang/test/CodeGenHLSL/validator_version.hlsl new file mode 100644 index 0000000000000..6ed9788043bfc --- /dev/null +++ b/clang/test/CodeGenHLSL/validator_version.hlsl @@ -0,0 +1,10 @@ +// RUN: %clang -cc1 -S -triple dxil-pc-shadermodel6.3-library -S -emit-llvm -xhlsl -validator-version 1.1 -o - %s | FileCheck %s + +// CHECK:!"dx.valver", ![[valver:[0-9]+]]} +// CHECK:![[valver]] = !{i32 1, i32 1} + +float bar(float a, float b); + +float foo(float a, float b) { + return bar(a, b); +} \ No newline at end of file diff --git a/clang/unittests/Driver/ToolChainTest.cpp b/clang/unittests/Driver/ToolChainTest.cpp index 8332143fa2931..cb17be3aca411 100644 --- a/clang/unittests/Driver/ToolChainTest.cpp +++ b/clang/unittests/Driver/ToolChainTest.cpp @@ -367,4 +367,112 @@ TEST(GetDriverMode, PrefersLastDriverMode) { EXPECT_EQ(getDriverMode(Args[0], llvm::makeArrayRef(Args).slice(1)), "bar"); } +TEST(DxcModeTest, ValidatorVersionValidation) { + IntrusiveRefCntPtr DiagID(new DiagnosticIDs()); + struct SimpleDiagnosticConsumer : public DiagnosticConsumer { + void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, + const Diagnostic &Info) override { + if (DiagLevel == DiagnosticsEngine::Level::Error) { + Errors.emplace_back(); + Info.FormatDiagnostic(Errors.back()); + Errors.back().append({0}); + } else { + Msgs.emplace_back(); + Info.FormatDiagnostic(Msgs.back()); + Msgs.back().append({0}); + } + } + void clear() override { + Msgs.clear(); + Errors.clear(); + DiagnosticConsumer::clear(); + } + std::vector> Msgs; + std::vector> Errors; + }; + + IntrusiveRefCntPtr InMemoryFileSystem( + new llvm::vfs::InMemoryFileSystem); + + InMemoryFileSystem->addFile("foo.hlsl", 0, + llvm::MemoryBuffer::getMemBuffer("\n")); + + auto *DiagConsumer = new SimpleDiagnosticConsumer; + IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions(); + DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagConsumer); + Driver TheDriver("/bin/clang", "", Diags, "", InMemoryFileSystem); + std::unique_ptr C( + TheDriver.BuildCompilation({"clang", "--driver-mode=dxc", "foo.hlsl"})); + EXPECT_TRUE(C); + EXPECT_TRUE(!C->containsError()); + + auto &TC = C->getDefaultToolChain(); + bool ContainsError = false; + auto Args = TheDriver.ParseArgStrings({"-validator-version", "1.1"}, false, + ContainsError); + EXPECT_FALSE(ContainsError); + auto DAL = std::make_unique(Args); + for (auto *A : Args) { + DAL->append(A); + } + auto *TranslatedArgs = + TC.TranslateArgs(*DAL, "0", Action::OffloadKind::OFK_None); + EXPECT_NE(TranslatedArgs, nullptr); + if (TranslatedArgs) { + auto *A = TranslatedArgs->getLastArg( + clang::driver::options::OPT_dxil_validator_version); + EXPECT_NE(A, nullptr); + if (A) { + EXPECT_STREQ(A->getValue(), "1.1"); + } + } + EXPECT_EQ(Diags.getNumErrors(), 0); + + // Invalid tests. + Args = TheDriver.ParseArgStrings({"-validator-version", "0.1"}, false, + ContainsError); + EXPECT_FALSE(ContainsError); + DAL = std::make_unique(Args); + for (auto *A : Args) { + DAL->append(A); + } + TranslatedArgs = TC.TranslateArgs(*DAL, "0", Action::OffloadKind::OFK_None); + EXPECT_EQ(Diags.getNumErrors(), 1); + EXPECT_STREQ(DiagConsumer->Errors.back().data(), + "invalid validator version : 0.1\nIf validator major version is " + "0, minor version must also be 0."); + Diags.Clear(); + DiagConsumer->clear(); + + Args = TheDriver.ParseArgStrings({"-validator-version", "2.1"}, false, + ContainsError); + EXPECT_FALSE(ContainsError); + DAL = std::make_unique(Args); + for (auto *A : Args) { + DAL->append(A); + } + TranslatedArgs = TC.TranslateArgs(*DAL, "0", Action::OffloadKind::OFK_None); + EXPECT_EQ(Diags.getNumErrors(), 2); + EXPECT_STREQ(DiagConsumer->Errors.back().data(), + "invalid validator version : 2.1\nValidator version must be " + "less than or equal to current internal version."); + Diags.Clear(); + DiagConsumer->clear(); + + Args = TheDriver.ParseArgStrings({"-validator-version", "1"}, false, + ContainsError); + EXPECT_FALSE(ContainsError); + DAL = std::make_unique(Args); + for (auto *A : Args) { + DAL->append(A); + } + TranslatedArgs = TC.TranslateArgs(*DAL, "0", Action::OffloadKind::OFK_None); + EXPECT_EQ(Diags.getNumErrors(), 3); + EXPECT_STREQ(DiagConsumer->Errors.back().data(), + "invalid validator version : 1\nFormat of validator version is " + "\".\" (ex:\"1.4\")."); + Diags.Clear(); + DiagConsumer->clear(); +} + } // end anonymous namespace. From 031ef95650970d2f03ac095863a7eb189b429f70 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Tue, 5 Apr 2022 14:08:28 -0700 Subject: [PATCH 2/4] Code cleanup. --- .../clang/Basic/DiagnosticDriverKinds.td | 11 ++++++-- clang/lib/CodeGen/CGHLSLRuntime.cpp | 4 --- clang/lib/CodeGen/CGHLSLRuntime.h | 15 ++++++----- clang/lib/Driver/ToolChains/HLSL.cpp | 27 +++++++++---------- 4 files changed, 30 insertions(+), 27 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index f41da15888499..dbbea65fe9e25 100644 --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -657,6 +657,13 @@ def err_drv_target_variant_invalid : Error< def err_drv_invalid_directx_shader_module : Error< "invalid profile : %0">; -def err_drv_invalid_dxil_validator_version : Error< - "invalid validator version : %0\n%1">; +def err_drv_invalid_range_dxil_validator_version : Error< + "invalid validator version : %0\n" + "Validator version must be less than or equal to current internal version.">; +def err_drv_invalid_format_dxil_validator_version : Error< + "invalid validator version : %0\n" + "Format of validator version is \".\" (ex:\"1.4\").">; +def err_drv_invalid_empty_dxil_validator_version : Error< + "invalid validator version : %0\n" + "If validator major version is 0, minor version must also be 0.">; } diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp index 75373a12993cf..84ed2ee1c0c8c 100644 --- a/clang/lib/CodeGen/CGHLSLRuntime.cpp +++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp @@ -44,10 +44,6 @@ void addDxilValVersion(StringRef ValVersionStr, llvm::Module &M) { } } // namespace -CGHLSLRuntime::CGHLSLRuntime(CodeGenModule &CGM) : CGM(CGM) {} - -CGHLSLRuntime::~CGHLSLRuntime() {} - void CGHLSLRuntime::finishCodeGen() { auto &CGOpts = CGM.getCodeGenOpts(); llvm::Module &M = CGM.getModule(); diff --git a/clang/lib/CodeGen/CGHLSLRuntime.h b/clang/lib/CodeGen/CGHLSLRuntime.h index 43581a36a320f..fbace4b3e5d0b 100644 --- a/clang/lib/CodeGen/CGHLSLRuntime.h +++ b/clang/lib/CodeGen/CGHLSLRuntime.h @@ -12,7 +12,8 @@ // //===----------------------------------------------------------------------===// -#pragma once +#ifndef LLVM_CLANG_LIB_CODEGEN_CGHLSLRUNTIME_H +#define LLVM_CLANG_LIB_CODEGEN_CGHLSLRUNTIME_H namespace clang { @@ -25,11 +26,13 @@ class CGHLSLRuntime { CodeGenModule &CGM; public: - CGHLSLRuntime(CodeGenModule &CGM); - virtual ~CGHLSLRuntime(); - void finishCodeGen(); + CGHLSLRuntime(CodeGenModule &CGM) : CGM(CGM) {} + virtual ~CGHLSLRuntime() {} + void finishCodeGen(); }; -} -} +} // namespace CodeGen +} // namespace clang + +#endif \ No newline at end of file diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp index 7a2791a89af53..166f3831adc26 100644 --- a/clang/lib/Driver/ToolChains/HLSL.cpp +++ b/clang/lib/Driver/ToolChains/HLSL.cpp @@ -126,25 +126,25 @@ std::string tryParseProfile(StringRef Profile) { return ""; } -bool isLegalValidatorVersion(StringRef ValVersionStr, std::string &ErrorMsg) { +bool isLegalValidatorVersion(StringRef ValVersionStr, const Driver &D) { auto VerPair = ValVersionStr.split("."); llvm::APInt APMajor, APMinor; - - if (VerPair.first.getAsInteger(0, APMajor) || - VerPair.second.getAsInteger(0, APMinor)) { - ErrorMsg = - "Format of validator version is \".\" (ex:\"1.4\")."; + VersionTuple Version; + if (Version.tryParse(ValVersionStr) || Version.getBuild() || + Version.getSubminor() || !Version.getMinor()) { + D.Diag(diag::err_drv_invalid_format_dxil_validator_version) + << ValVersionStr; return false; } - uint64_t Major = APMajor.getLimitedValue(); - uint64_t Minor = APMinor.getLimitedValue(); + + uint64_t Major = Version.getMajor(); + uint64_t Minor = Version.getMinor().getValue(); if (Major > MaxDXILMajor || (Major == MaxDXILMajor && Minor > MaxDXILMinor)) { - ErrorMsg = "Validator version must be less than or equal to current " - "internal version."; + D.Diag(diag::err_drv_invalid_range_dxil_validator_version) << ValVersionStr; return false; } if (Major == 0 && Minor != 0) { - ErrorMsg = "If validator major version is 0, minor version must also be 0."; + D.Diag(diag::err_drv_invalid_empty_dxil_validator_version) << ValVersionStr; return false; } return true; @@ -185,10 +185,7 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch, if (A->getOption().getID() == options::OPT_dxil_validator_version) { StringRef ValVerStr = A->getValue(); std::string ErrorMsg; - if (!isLegalValidatorVersion(ValVerStr, ErrorMsg)) { - getDriver().Diag(diag::err_drv_invalid_dxil_validator_version) - << ValVerStr << ErrorMsg; - + if (!isLegalValidatorVersion(ValVerStr, getDriver())) { continue; } } From 4a67a3b98d05577f47ac18ed677e401082bc9e11 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Tue, 5 Apr 2022 15:44:16 -0700 Subject: [PATCH 3/4] Remove dead code. --- clang/lib/Driver/ToolChains/HLSL.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp index 166f3831adc26..7dc9236a7ee40 100644 --- a/clang/lib/Driver/ToolChains/HLSL.cpp +++ b/clang/lib/Driver/ToolChains/HLSL.cpp @@ -127,8 +127,6 @@ std::string tryParseProfile(StringRef Profile) { } bool isLegalValidatorVersion(StringRef ValVersionStr, const Driver &D) { - auto VerPair = ValVersionStr.split("."); - llvm::APInt APMajor, APMinor; VersionTuple Version; if (Version.tryParse(ValVersionStr) || Version.getBuild() || Version.getSubminor() || !Version.getMinor()) { From 437889a6abf72ba7fd1449c78490d23199bd2668 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Tue, 5 Apr 2022 23:40:47 -0700 Subject: [PATCH 4/4] Code cleanup. --- clang/lib/CodeGen/CGHLSLRuntime.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp index 84ed2ee1c0c8c..a1920df1df1ed 100644 --- a/clang/lib/CodeGen/CGHLSLRuntime.cpp +++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp @@ -26,15 +26,15 @@ namespace { void addDxilValVersion(StringRef ValVersionStr, llvm::Module &M) { // The validation of ValVersionStr is done at HLSLToolChain::TranslateArgs. // Assume ValVersionStr is legal here. - auto VerPair = ValVersionStr.split("."); - llvm::APInt APMajor, APMinor; - - if (VerPair.first.getAsInteger(0, APMajor) || - VerPair.second.getAsInteger(0, APMinor)) { + VersionTuple Version; + if (Version.tryParse(ValVersionStr) || Version.getBuild() || + Version.getSubminor() || !Version.getMinor()) { return; } - uint64_t Major = APMajor.getLimitedValue(); - uint64_t Minor = APMinor.getLimitedValue(); + + uint64_t Major = Version.getMajor(); + uint64_t Minor = Version.getMinor().getValue(); + auto &Ctx = M.getContext(); IRBuilder<> B(M.getContext()); MDNode *Val = MDNode::get(Ctx, {ConstantAsMetadata::get(B.getInt32(Major)),