[llvm-branch-commits] [DXIL][Analysis] Boilerplate for DXILResourceAnalysis pass (PR #100700)
@@ -212,6 +216,53 @@ class ResourceInfo {
};
} // namespace dxil
+
+using DXILResourceMap = MapVector;
+
+class DXILResourceAnalysis : public AnalysisInfoMixin {
+ friend AnalysisInfoMixin;
+
+ static AnalysisKey Key;
+
+public:
+ using Result = DXILResourceMap;
+
+ /// Gather resource info for the module \c M.
+ DXILResourceMap run(Module &M, ModuleAnalysisManager &AM);
llvm-beanz wrote:
No, you query the analysis manager which manages caching and invalidating the
results. Something like:
```
AM.getResult()
```
https://github.com/llvm/llvm-project/pull/100700
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DXIL][Analysis] Boilerplate for DXILResourceAnalysis pass (PR #100700)
https://github.com/llvm-beanz approved this pull request. https://github.com/llvm/llvm-project/pull/100700 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [HLSL] RWBuffer should not have a default parameter (PR #71265)
https://github.com/llvm-beanz approved this pull request. https://github.com/llvm/llvm-project/pull/71265 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] fix: added to fix compile error (PR #67217)
https://github.com/llvm-beanz closed https://github.com/llvm/llvm-project/pull/67217 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] fix: added to fix compile error (PR #67217)
llvm-beanz wrote: Hi @isubasinghe, as you guessed LLVM 9 is a closed release so we're not accepting PRs into it anymore (even ones that fix pretty obvious issues). Thank you for your contribution (even though we won't be merging it). https://github.com/llvm/llvm-project/pull/67217 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [HLSL] Add helpers to simplify HLSL resource type declarations. NFC (PR #73967)
https://github.com/llvm-beanz approved this pull request. https://github.com/llvm/llvm-project/pull/73967 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [DirectX] Move DXIL ResourceKind and ElementType to DXILABI.h. NFC (PR #78225)
https://github.com/llvm-beanz approved this pull request. Looks like you angered the clang-format god, but otherwise LGTM. https://github.com/llvm/llvm-project/pull/78225 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.typedBufferLoad` to DXIL ops (PR #104252)
@@ -683,6 +686,17 @@ def CreateHandle : DXILOp<57, createHandle> {
let stages = [Stages];
}
+def BufferLoad : DXILOp<68, bufferLoad> {
+ let Doc = "reads from a TypedBuffer";
+ // Handle, Coord0, Coord1
+ let arguments = [HandleTy, Int32Ty, Int32Ty];
+ let result = OverloadTy;
+ let overloads =
+ [Overloads];
llvm-beanz wrote:
The 16-bit overloads were always valid in DXIL 1.0, but they didn't actually
mean 16-bit types, they meant the min16{float|int|uint} types. This is one of
the things that's really wonky about DXIL defining interpretations of LLVM IR
that conflicted with LLVM's core definition.
I think the code here is accurate to what we need for that.
https://github.com/llvm/llvm-project/pull/104252
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.typedBufferLoad` to DXIL ops (PR #104252)
https://github.com/llvm-beanz approved this pull request. https://github.com/llvm/llvm-project/pull/104252 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.typedBufferLoad` to DXIL ops (PR #104252)
llvm-beanz wrote: All looks good to me. https://github.com/llvm/llvm-project/pull/104252 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [DirectX] Implement the resource.load.rawbuffer intrinsic (PR #121012)
@@ -542,6 +542,48 @@ class OpLowerer {
});
}
+ [[nodiscard]] bool lowerRawBufferLoad(Function &F) {
+Triple TT(Triple(M.getTargetTriple()));
+VersionTuple DXILVersion = TT.getDXILVersion();
+const DataLayout &DL = F.getDataLayout();
+IRBuilder<> &IRB = OpBuilder.getIRB();
+Type *Int8Ty = IRB.getInt8Ty();
+Type *Int32Ty = IRB.getInt32Ty();
+
+return replaceFunction(F, [&](CallInst *CI) -> Error {
+ IRB.SetInsertPoint(CI);
+
+ Type *OldTy = cast(CI->getType())->getElementType(0);
+ Type *ScalarTy = OldTy->getScalarType();
+ Type *NewRetTy = OpBuilder.getResRetType(ScalarTy);
+
+ Value *Handle =
+ createTmpHandleCast(CI->getArgOperand(0), OpBuilder.getHandleType());
+ Value *Index0 = CI->getArgOperand(1);
+ Value *Index1 = CI->getArgOperand(2);
+ uint64_t NumElements =
+ DL.getTypeSizeInBits(OldTy) / DL.getTypeSizeInBits(ScalarTy);
+ Value *Mask = ConstantInt::get(Int8Ty, ~(~0U << NumElements));
+ Value *Align =
+ ConstantInt::get(Int32Ty, DL.getPrefTypeAlign(ScalarTy).value());
+
+ Expected OpCall =
+ DXILVersion >= VersionTuple(1, 2)
+ ? OpBuilder.tryCreateOp(OpCode::RawBufferLoad,
+ {Handle, Index0, Index1, Mask, Align},
+ CI->getName(), NewRetTy)
+ : OpBuilder.tryCreateOp(OpCode::BufferLoad,
+ {Handle, Index0, Index1}, CI->getName(),
+ NewRetTy);
+ if (Error E = OpCall.takeError())
+return E;
llvm-beanz wrote:
nit: personal preference, take or leave it.
```suggestion
if (!OpCall)
return OpCall.takeError();
```
https://github.com/llvm/llvm-project/pull/121012
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [DirectX] Implement the resource.load.rawbuffer intrinsic (PR #121012)
https://github.com/llvm-beanz approved this pull request. LGTM. One extremely small nit that you can take or leave. https://github.com/llvm/llvm-project/pull/121012 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [DirectX] Implement the resource.load.rawbuffer intrinsic (PR #121012)
https://github.com/llvm-beanz edited https://github.com/llvm/llvm-project/pull/121012 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [DirectX] Implement the resource.store.rawbuffer intrinsic (PR #121282)
https://github.com/llvm-beanz approved this pull request. https://github.com/llvm/llvm-project/pull/121282 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [DirectX] Split resource info into type and binding info. NFC (PR #119773)
https://github.com/llvm-beanz approved this pull request. https://github.com/llvm/llvm-project/pull/119773 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [DirectX] Lower ops after translating metadata (PR #120157)
https://github.com/llvm-beanz approved this pull request. https://github.com/llvm/llvm-project/pull/120157 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Implement Parsing of Descriptor Tables (PR #122982)
https://github.com/llvm-beanz commented: I think that the way you're breaking up this change is sub-optimal from a review perspective. You've added a lot of code that partially handles parsing a very complex root signature. The problem is that to complete this implementation you're going to go back over this code over and over again fleshing it out, and from a reviewer's perspective we're going to need to keep paging back in extra context. If instead you started with a much simpler root signature (even just an empty one), but implement more complete handling for it, we can review that and incrementally build up without revisiting the same code over and over again in each subsequent patch. https://github.com/llvm/llvm-project/pull/122982 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Implement Parsing of Descriptor Tables (PR #122982)
@@ -0,0 +1,140 @@
+//===- HLSLRootSignature.h - HLSL Root Signature helper objects
---===//
+//
+// 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
+//
+//===--===//
+///
+/// \file This file contains helper objects for working with HLSL Root
+/// Signatures.
+///
+//===--===//
+
+#ifndef LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H
+#define LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H
+
+#include
+
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/Endian.h"
+
+namespace llvm {
+namespace hlsl {
+namespace root_signature {
+
+// This is a copy from DebugInfo/CodeView/CodeView.h
+#define RS_DEFINE_ENUM_CLASS_FLAGS_OPERATORS(Class)
\
+ inline Class operator|(Class a, Class b) {
\
+return static_cast(llvm::to_underlying(a) |
\
+ llvm::to_underlying(b));
\
+ }
\
+ inline Class operator&(Class a, Class b) {
\
+return static_cast(llvm::to_underlying(a) &
\
+ llvm::to_underlying(b));
\
+ }
\
+ inline Class operator~(Class a) {
\
+return static_cast(~llvm::to_underlying(a));
\
+ }
\
+ inline Class &operator|=(Class &a, Class b) {
\
+a = a | b;
\
+return a;
\
+ }
\
+ inline Class &operator&=(Class &a, Class b) {
\
+a = a & b;
\
+return a;
\
+ }
+
+// Definition of the various enumerations and flags
+enum class DescriptorRangeFlags : unsigned {
+ None = 0,
+ DescriptorsVolatile = 0x1,
+ DataVolatile = 0x2,
+ DataStaticWhileSetAtExecute = 0x4,
+ DataStatic = 0x8,
+ DescriptorsStaticKeepingBufferBoundsChecks = 0x1,
+ ValidFlags = 0x1000f,
+ ValidSamplerFlags = DescriptorsVolatile,
+};
+RS_DEFINE_ENUM_CLASS_FLAGS_OPERATORS(DescriptorRangeFlags)
+
+enum class ShaderVisibility {
+ All = 0,
+ Vertex = 1,
+ Hull = 2,
+ Domain = 3,
+ Geometry = 4,
+ Pixel = 5,
+ Amplification = 6,
+ Mesh = 7,
+};
+
+// Definitions of the in-memory data layout structures
+
+// Models the different registers: bReg | tReg | uReg | sReg
+enum class RegisterType { BReg, TReg, UReg, SReg };
+struct Register {
+ RegisterType ViewType;
+ uint32_t Number;
+};
+
+static const uint32_t DescriptorTableOffsetAppend = 0x;
+// Models DTClause : CBV | SRV | UAV | Sampler by collecting like parameters
+enum class ClauseType { CBV, SRV, UAV, Sampler };
llvm-beanz wrote:
Can we do this instead?
```suggestion
using ClauseType = llvm::dxil::ResourceClass
```
This will change `CBV` to `CBuffer`, but otherwise those enums need to be the
same right?
https://github.com/llvm/llvm-project/pull/122982
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Implement Parsing of Descriptor Tables (PR #122982)
@@ -89,6 +91,72 @@ class RootSignatureLexer {
}
};
+class RootSignatureParser {
+public:
+ RootSignatureParser(SmallVector &Elements,
+ const SmallVector &Tokens);
+
+ // Iterates over the provided tokens and constructs the in-memory
+ // representations of the RootElements.
+ //
+ // The return value denotes if there was a failure and the method will
+ // return on the first encountered failure, or, return false if it
+ // can sucessfully reach the end of the tokens.
+ bool Parse();
+
+private:
+ bool ReportError(); // TODO: Implement this to report error through Diags
llvm-beanz wrote:
I don't think this should be separate. It's going to be really hard to ensure
that any follow-up that adds error reporting properly covers all the cases.
https://github.com/llvm/llvm-project/pull/122982
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Implement Parsing of Descriptor Tables (PR #122982)
https://github.com/llvm-beanz edited https://github.com/llvm/llvm-project/pull/122982 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] compiler-rt: Introduce runtime functions for emulated PAC. (PR #133530)
@@ -0,0 +1,7343 @@ +/* + * xxHash - Extremely Fast Hash algorithm + * Header File + * Copyright (C) 2012-2023 Yann Collet + * + * BSD 2-Clause License (https://www.opensource.org/licenses/bsd-license.php) llvm-beanz wrote: The introduction of BSD-licensed code to compiler-rt (and especially to compiler-rt's builtins library) poses significant licensing difficulty. The BSD 2-clause license requires attribution when code is distributed in object form as well as source form, and since the compiler-rt builtins are linked into binaries _built with_ LLVM (not just LLVM itself), the impact of the license here would be transitive to products built using LLVM-based compilers. This is extremely undesirable, and as such this change will need to replace the xxhash with an alternative hash (ideally under the LLVM license) in order to be integrated. https://github.com/llvm/llvm-project/pull/133530 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] compiler-rt: Introduce runtime functions for emulated PAC. (PR #133530)
https://github.com/llvm-beanz requested changes to this pull request. We cannot introduce BSD-licensed code to compiler-rt. Please see [my comment for more details](https://github.com/llvm/llvm-project/pull/133530#discussion_r2021952691). https://github.com/llvm/llvm-project/pull/133530 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [DirectX] Fix printing of DXIL cbuffer info (PR #128698)
https://github.com/llvm-beanz approved this pull request. https://github.com/llvm/llvm-project/pull/128698 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [DirectX] Support the CBufferLoadLegacy operation (PR #128699)
https://github.com/llvm-beanz approved this pull request. https://github.com/llvm/llvm-project/pull/128699 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Define and integrate rootsig clang attr and decl (PR #137690)
https://github.com/llvm-beanz approved this pull request. https://github.com/llvm/llvm-project/pull/137690 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [DirectX] Improve error accumulation in root signature parsing (PR #144465)
@@ -699,19 +736,20 @@ static bool verifyBorderColor(uint32_t BorderColor) {
static bool verifyLOD(float LOD) { return !std::isnan(LOD); }
static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) {
-
+ bool HasError = false;
if (!verifyVersion(RSD.Version)) {
-return reportValueError(Ctx, "Version", RSD.Version);
+HasError = reportValueError(Ctx, "Version", RSD.Version) || HasError;
llvm-beanz wrote:
This pattern seems really awkward because the report functions always return
`true`. That means that the `||` is always unnecessary, but also the assignment
itself isn't really as clear to understand as it could be.
You could instead write this code as something like:
```
if (!verifyVersion(RSD.Version)) {
reportValueError(...);
HasError = true;
}
```
This is more verbose but a lot easier to see what is happening.
https://github.com/llvm/llvm-project/pull/144465
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [DirectX] Improve error handling and validation in root signature parsing (PR #144577)
@@ -48,6 +48,71 @@ static bool reportValueError(LLVMContext *Ctx, Twine
ParamName,
return true;
}
+// Template function to get formatted type string based on C++ type
+template std::string getTypeFormatted() {
llvm-beanz wrote:
Looks like this could be a StringRef.
```suggestion
template StringRef getTypeFormatted() {
```
https://github.com/llvm/llvm-project/pull/144577
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [DirectX] Improve error handling and validation in root signature parsing (PR #144577)
@@ -48,6 +48,71 @@ static bool reportValueError(LLVMContext *Ctx, Twine
ParamName,
return true;
}
+// Template function to get formatted type string based on C++ type
+template std::string getTypeFormatted() {
+ if constexpr (std::is_same_v) {
+return "string";
+ } else if constexpr (std::is_same_v ||
+ std::is_same_v) {
+return "metadata";
+ } else if constexpr (std::is_same_v ||
+ std::is_same_v) {
+return "constant";
+ } else if constexpr (std::is_same_v) {
+return "constant";
+ } else if constexpr (std::is_same_v ||
+ std::is_same_v) {
+return "constant int";
+ } else if constexpr (std::is_same_v) {
+return "constant int";
+ }
+ return "unknown";
+}
+
+// Helper function to get the actual type of a metadata operand
+std::string getActualMDType(const MDNode *Node, unsigned Index) {
llvm-beanz wrote:
```suggestion
StringRef getActualMDType(const MDNode *Node, unsigned Index) {
```
https://github.com/llvm/llvm-project/pull/144577
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [DirectX] Improve error handling and validation in root signature parsing (PR #144577)
@@ -48,6 +48,71 @@ static bool reportValueError(LLVMContext *Ctx, Twine
ParamName,
return true;
}
+// Template function to get formatted type string based on C++ type
+template std::string getTypeFormatted() {
+ if constexpr (std::is_same_v) {
+return "string";
+ } else if constexpr (std::is_same_v ||
+ std::is_same_v) {
+return "metadata";
+ } else if constexpr (std::is_same_v ||
+ std::is_same_v) {
+return "constant";
+ } else if constexpr (std::is_same_v) {
+return "constant";
+ } else if constexpr (std::is_same_v ||
+ std::is_same_v) {
+return "constant int";
+ } else if constexpr (std::is_same_v) {
+return "constant int";
+ }
+ return "unknown";
+}
+
+// Helper function to get the actual type of a metadata operand
+std::string getActualMDType(const MDNode *Node, unsigned Index) {
+ if (!Node || Index >= Node->getNumOperands())
+return "null";
+
+ Metadata *Op = Node->getOperand(Index);
+ if (!Op)
+return "null";
+
+ if (isa(Op))
+return getTypeFormatted();
+
+ if (isa(Op)) {
+if (auto *CAM = dyn_cast(Op)) {
+ Type *T = CAM->getValue()->getType();
+ if (T->isIntegerTy())
+return (Twine("i") + Twine(T->getIntegerBitWidth())).str();
+ if (T->isFloatingPointTy())
+return T->isFloatTy()? getTypeFormatted()
+ : T->isDoubleTy() ? getTypeFormatted()
+ : "fp";
+
+ return getTypeFormatted();
+}
+ }
+ if (isa(Op))
+return getTypeFormatted();
+
+ return "unknown";
+}
+
+// Helper function to simplify error reporting for invalid metadata values
+template
+auto reportInvalidTypeError(LLVMContext *Ctx, Twine ParamName,
llvm-beanz wrote:
`auto` as a return type rarely meets LLVM's coding standards:
see:
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
https://github.com/llvm/llvm-project/pull/144577
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [DirectX] Improve error accumulation in root signature parsing (PR #144465)
llvm-beanz wrote: Looking at this PR and https://github.com/llvm/llvm-project/pull/144577 led me to look a bit more at the code around this. I think the way you're handling errors in this code is a bit sub-optimal. Instead of having a bunch of report functions and chains of functions returning `bool`, you could instead have the functions return `llvm::Error` and use `joinErrors` to build up chains of errors which you can then choose to report at a higher level in the API. The LLVM Programmers Manual has a good section about `llvm::Error`: https://llvm.org/docs/ProgrammersManual.html#id23 https://github.com/llvm/llvm-project/pull/144465 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [DirectX] Add Range Overlap validation to `DXILPostOptimizationValidation.cpp` (PR #148919)
@@ -295,6 +297,105 @@ getRootSignature(RootSignatureBindingInfo &RSBI,
return RootSigDesc;
}
+static void
+reportOverlappingRegisters(Module &M,
+ llvm::hlsl::rootsig::OverlappingRanges Overlap) {
+ const llvm::hlsl::rootsig::RangeInfo *Info = Overlap.A;
+ const llvm::hlsl::rootsig::RangeInfo *OInfo = Overlap.B;
+ SmallString<128> Message;
+ raw_svector_ostream OS(Message);
+ OS << "register " << ResourceClassToString(Info->Class)
+ << " (space=" << Info->Space << ", register=" << Info->LowerBound << ")"
+ << " is overlapping with"
+ << " register " << ResourceClassToString(OInfo->Class)
+ << " (space=" << OInfo->Space << ", register=" << OInfo->LowerBound << ")"
+ << ", verify your root signature definition.";
+
+ M.getContext().diagnose(DiagnosticInfoGeneric(Message));
+}
+
+static bool reportOverlappingRanges(Module &M,
+const mcdxbc::RootSignatureDesc &RSD) {
+ using namespace llvm::hlsl::rootsig;
+
+ llvm::SmallVector Infos;
+
+ for (size_t I = 0; I < RSD.ParametersContainer.size(); I++) {
+const auto &[Type, Loc] =
+RSD.ParametersContainer.getTypeAndLocForParameter(I);
+const auto &Header = RSD.ParametersContainer.getHeader(I);
+switch (Type) {
+case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): {
llvm-beanz wrote:
Rather than using `llvm::to_underlying` if you convert the int to the enum
early (and handle the case where it doesn't match), the compiler will make sure
you have full switch coverage.
That's definitely the better pattern to use for cases like this.
https://github.com/llvm/llvm-project/pull/148919
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [DirectX] Add Range Overlap validation to `DXILPostOptimizationValidation.cpp` (PR #148919)
@@ -295,6 +297,105 @@ getRootSignature(RootSignatureBindingInfo &RSBI,
return RootSigDesc;
}
+static void
+reportOverlappingRegisters(Module &M,
+ llvm::hlsl::rootsig::OverlappingRanges Overlap) {
+ const llvm::hlsl::rootsig::RangeInfo *Info = Overlap.A;
+ const llvm::hlsl::rootsig::RangeInfo *OInfo = Overlap.B;
+ SmallString<128> Message;
+ raw_svector_ostream OS(Message);
+ OS << "register " << ResourceClassToString(Info->Class)
+ << " (space=" << Info->Space << ", register=" << Info->LowerBound << ")"
+ << " is overlapping with"
+ << " register " << ResourceClassToString(OInfo->Class)
+ << " (space=" << OInfo->Space << ", register=" << OInfo->LowerBound << ")"
+ << ", verify your root signature definition.";
+
+ M.getContext().diagnose(DiagnosticInfoGeneric(Message));
+}
+
+static bool reportOverlappingRanges(Module &M,
+const mcdxbc::RootSignatureDesc &RSD) {
+ using namespace llvm::hlsl::rootsig;
+
+ llvm::SmallVector Infos;
+
+ for (size_t I = 0; I < RSD.ParametersContainer.size(); I++) {
+const auto &[Type, Loc] =
+RSD.ParametersContainer.getTypeAndLocForParameter(I);
+const auto &Header = RSD.ParametersContainer.getHeader(I);
+switch (Type) {
+case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): {
llvm-beanz wrote:
I filed an issue to clean up this pattern across the DX backend. We really
shouldn't be writing switch statements like this. The issue is
https://github.com/llvm/llvm-project/issues/150676.
https://github.com/llvm/llvm-project/pull/148919
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Add parsing of floats for StaticSampler (PR #140181)
@@ -711,6 +734,35 @@ std::optional
RootSignatureParser::parseRegister() {
return Reg;
}
+std::optional RootSignatureParser::parseFloatParam() {
+ assert(CurToken.TokKind == TokenKind::pu_equal &&
+ "Expects to only be invoked starting at given keyword");
+ // Consume sign modifier
+ bool Signed =
+ tryConsumeExpectedToken({TokenKind::pu_plus, TokenKind::pu_minus});
+ bool Negated = Signed && CurToken.TokKind == TokenKind::pu_minus;
+
+ // DXC will treat a postive signed integer as unsigned
+ if (!Negated && tryConsumeExpectedToken(TokenKind::int_literal)) {
+auto UInt = handleUIntLiteral();
+if (!UInt.has_value())
+ return std::nullopt;
+return (float)UInt.value();
+ } else if (tryConsumeExpectedToken(TokenKind::int_literal)) {
llvm-beanz wrote:
Flyby style nit: Don't use `else` after a `return`
https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
https://github.com/llvm/llvm-project/pull/140181
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [HLSL][RootSignature] Implement serialization of remaining Root Elements (PR #143198)
@@ -71,6 +71,199 @@ static raw_ostream &operator<<(raw_ostream &OS,
return OS;
}
+static raw_ostream &operator<<(raw_ostream &OS, const SamplerFilter &Filter) {
+ switch (Filter) {
+ case SamplerFilter::MinMagMipPoint:
+OS << "MinMagMipPoint";
llvm-beanz wrote:
Could we instead use `llvm::EnumEntry` from `llvm::ScopedPrinter` to translate
enums to strings? That allows the mechanism to be reused rather than single
purpose.
https://github.com/llvm/llvm-project/pull/143198
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [HLSL][DirectX] Add the Qdx-rootsignature-strip driver option (PR #154454)
@@ -76,9 +76,10 @@ class Action {
StaticLibJobClass,
BinaryAnalyzeJobClass,
BinaryTranslatorJobClass,
+BinaryModifyJobClass,
llvm-beanz wrote:
Probably better to align this with the specific tool you're running:
```suggestion
ObjcopyJobClass,
```
That is consistent with most of the other toolchain jobs (e..g. LipoJobClass,
DsymutilJobClass, LinkerWrapperJobClass, etc).
https://github.com/llvm/llvm-project/pull/154454
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [HLSL][DirectX] Add the Qdx-rootsignature-strip driver option (PR #154454)
@@ -247,6 +247,30 @@ void tools::hlsl::MetalConverter::ConstructJob(
Exec, CmdArgs, Inputs, Input));
}
+void tools::hlsl::LLVMObjcopy::ConstructJob(Compilation &C, const JobAction
&JA,
+const InputInfo &Output,
+const InputInfoList &Inputs,
+const ArgList &Args,
+const char *LinkingOutput) const {
+
+ std::string ObjcopyPath = getToolChain().GetProgramPath("llvm-objcopy");
+ const char *Exec = Args.MakeArgString(ObjcopyPath);
+
+ ArgStringList CmdArgs;
+ assert(Inputs.size() == 1 && "Unable to handle multiple inputs.");
+ const InputInfo &Input = Inputs[0];
+ CmdArgs.push_back(Input.getFilename());
+ CmdArgs.push_back(Output.getFilename());
+
+ if (Args.hasArg(options::OPT_dxc_strip_rootsignature)) {
+const char *Frs = Args.MakeArgString("--remove-section=RTS0");
+CmdArgs.push_back(Frs);
+ }
+
llvm-beanz wrote:
Should we assert that `CmdArgs > 2` to ensure that we didn't just invoke
objcopy for no reason?
https://github.com/llvm/llvm-project/pull/154454
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [HLSL][DirectX] Add the Qdx-rootsignature-strip driver option (PR #154454)
@@ -4601,6 +4601,16 @@ void Driver::BuildActions(Compilation &C, DerivedArgList
&Args,
Actions.push_back(C.MakeAction(
LastAction, types::TY_DX_CONTAINER));
}
+if (TC.requiresObjcopy(Args)) {
+ Action *LastAction = Actions.back();
+ // llvm-objcopy expects a DXIL container, which can either be
+ // validated (in which case they are TY_DX_CONTAINER), or unvalidated
+ // (TY_OBJECT).
+ if (LastAction->getType() == types::TY_DX_CONTAINER ||
llvm-beanz wrote:
Should we be running this before validation rather than after? What does DXC do?
https://github.com/llvm/llvm-project/pull/154454
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [HLSL][DirectX] Add the Qdx-rootsignature-strip driver option (PR #154454)
@@ -42,6 +42,19 @@ class LLVM_LIBRARY_VISIBILITY MetalConverter : public Tool {
const llvm::opt::ArgList &TCArgs,
const char *LinkingOutput) const override;
};
+
+class LLVM_LIBRARY_VISIBILITY LLVMObjcopy : public Tool {
+public:
+ LLVMObjcopy(const ToolChain &TC)
+ : Tool("hlsl::LLVMObjcopy", "llvm-objcopy", TC) {}
llvm-beanz wrote:
I don't think the `hlsl` is relevant here. It's really just objcopy.
```suggestion
: Tool("LLVMObjcopy", "llvm-objcopy", TC) {}
```
https://github.com/llvm/llvm-project/pull/154454
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [HLSL] Use static create methods to initialize resources in arrays (PR #157005)
@@ -110,52 +112,29 @@ static int getTotalArraySize(ASTContext &AST, const
clang::Type *Ty) {
return AST.getConstantArrayElementCount(cast(Ty));
}
-// Find constructor decl for a specific resource record type and binding
-// (implicit vs. explicit). The constructor has 5 parameters.
-// For explicit binding the signature is:
-// void(unsigned, unsigned, int, unsigned, const char *).
-// For implicit binding the signature is:
-// void(unsigned, int, unsigned, unsigned, const char *).
-static CXXConstructorDecl *findResourceConstructorDecl(ASTContext &AST,
- QualType ResTy,
- bool ExplicitBinding) {
- std::array ExpParmTypes = {
- AST.UnsignedIntTy, AST.UnsignedIntTy, AST.UnsignedIntTy,
- AST.UnsignedIntTy, AST.getPointerType(AST.CharTy.withConst())};
- ExpParmTypes[ExplicitBinding ? 2 : 1] = AST.IntTy;
-
- CXXRecordDecl *ResDecl = ResTy->getAsCXXRecordDecl();
- for (auto *Ctor : ResDecl->ctors()) {
-if (Ctor->getNumParams() != ExpParmTypes.size())
- continue;
-auto *ParmIt = Ctor->param_begin();
-auto ExpTyIt = ExpParmTypes.begin();
-for (; ParmIt != Ctor->param_end() && ExpTyIt != ExpParmTypes.end();
- ++ParmIt, ++ExpTyIt) {
- if ((*ParmIt)->getType() != *ExpTyIt)
-break;
-}
-if (ParmIt == Ctor->param_end())
- return Ctor;
- }
- llvm_unreachable("did not find constructor for resource class");
-}
-
static Value *buildNameForResource(llvm::StringRef BaseName,
CodeGenModule &CGM) {
llvm::SmallString<64> GlobalName = {BaseName, ".str"};
return CGM.GetAddrOfConstantCString(BaseName.str(), GlobalName.c_str())
.getPointer();
}
-static void createResourceCtorArgs(CodeGenModule &CGM, CXXConstructorDecl *CD,
- llvm::Value *ThisPtr, llvm::Value *Range,
- llvm::Value *Index, StringRef Name,
- HLSLResourceBindingAttr *RBA,
- HLSLVkBindingAttr *VkBinding,
- CallArgList &Args) {
+static CXXMethodDecl *lookupMethod(CXXRecordDecl *Record, StringRef Name,
llvm-beanz wrote:
We should probably do this with `Sema::LookupSingleName`.
https://github.com/llvm/llvm-project/pull/157005
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [HLSL] Use static create methods to initialize resources in arrays (PR #157005)
@@ -243,16 +238,18 @@ static Value *initializeLocalResourceArray( Index = CGF.Builder.CreateAdd(Index, One); GEPIndices.back() = llvm::ConstantInt::get(IntTy, I); } -Address ThisAddress = +Address ReturnAddress = CGF.Builder.CreateGEP(TmpArrayAddr, GEPIndices, Ty, Align); -llvm::Value *ThisPtr = CGF.getAsNaturalPointerTo(ThisAddress, ElemType); CallArgList Args; -createResourceCtorArgs(CGF.CGM, CD, ThisPtr, Range, Index, ResourceName, - RBA, VkBinding, Args); -CGF.EmitCXXConstructorCall(CD, Ctor_Complete, false, false, ThisAddress, - Args, ValueSlot.mayOverlap(), ArraySubsExprLoc, - ValueSlot.isSanitizerChecked()); +CXXMethodDecl *CreateMethod = lookupResourceInitMethodAndSetupArgs( +CGF.CGM, ResourceDecl, Range, Index, ResourceName, RBA, VkBinding, +Args); + +if (!CreateMethod) + return std::nullopt; llvm-beanz wrote: Do we need to surface an error if this happens? Can this happen? https://github.com/llvm/llvm-project/pull/157005 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [HLSL] Use static create methods to initialize resources in arrays (PR #157005)
@@ -961,27 +952,27 @@ std::optional
CGHLSLRuntime::emitResourceArraySubscriptExpr(
// If the result of the subscript operation is a single resource, call the
// constructor.
if (ResultTy == ResourceTy) {
-QualType ThisType = CD->getThisType()->getPointeeType();
-llvm::Value *ThisPtr = CGF.getAsNaturalPointerTo(TmpVarAddress, ThisType);
-
-// Assemble the constructor parameters.
CallArgList Args;
-createResourceCtorArgs(CGM, CD, ThisPtr, Range, Index,
ArrayDecl->getName(),
- RBA, VkBinding, Args);
-// Call the constructor.
-CGF.EmitCXXConstructorCall(CD, Ctor_Complete, false, false, TmpVarAddress,
- Args, ValueSlot.mayOverlap(),
- ArraySubsExpr->getExprLoc(),
- ValueSlot.isSanitizerChecked());
+CXXMethodDecl *CreateMethod = lookupResourceInitMethodAndSetupArgs(
+CGF.CGM, ResourceTy->getAsCXXRecordDecl(), Range, Index,
+ArrayDecl->getName(), RBA, VkBinding, Args);
+
+if (!CreateMethod)
+ return std::nullopt;
llvm-beanz wrote:
Same as above, what happens if this happens?
https://github.com/llvm/llvm-project/pull/157005
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [HLSL] Use static create methods to initialize resources in arrays (PR #157005)
https://github.com/llvm-beanz approved this pull request. https://github.com/llvm/llvm-project/pull/157005 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [HLSL] Use static create methods to initialize resources in arrays (PR #157005)
@@ -243,16 +238,18 @@ static Value *initializeLocalResourceArray( Index = CGF.Builder.CreateAdd(Index, One); GEPIndices.back() = llvm::ConstantInt::get(IntTy, I); } -Address ThisAddress = +Address ReturnAddress = CGF.Builder.CreateGEP(TmpArrayAddr, GEPIndices, Ty, Align); -llvm::Value *ThisPtr = CGF.getAsNaturalPointerTo(ThisAddress, ElemType); CallArgList Args; -createResourceCtorArgs(CGF.CGM, CD, ThisPtr, Range, Index, ResourceName, - RBA, VkBinding, Args); -CGF.EmitCXXConstructorCall(CD, Ctor_Complete, false, false, ThisAddress, - Args, ValueSlot.mayOverlap(), ArraySubsExprLoc, - ValueSlot.isSanitizerChecked()); +CXXMethodDecl *CreateMethod = lookupResourceInitMethodAndSetupArgs( +CGF.CGM, ResourceDecl, Range, Index, ResourceName, RBA, VkBinding, +Args); + +if (!CreateMethod) + return std::nullopt; llvm-beanz wrote: Chatted with @hekota offline. Seems like this happens for manually written structs containing handles, and the outcome here is that they just don't get bindings generated. I think not supporting automatically binding handles for people writing their own types against internal compiler details is completely reasonable. https://github.com/llvm/llvm-project/pull/157005 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
