Add support to convert DXR HLSL to SPV_NV_ray_tracing #1920

alelenv · 2019-02-11T23:53:32Z

@ehsannas @antiagainst
Ehsan, Lei, please review the changes

Pattern match tests have been added for new features

Let us know if anything more is required from us
Thanks!

msftclas · 2019-02-11T23:53:43Z

All CLA requirements met.

alelenv · 2019-02-12T00:13:57Z

GLSL Spec : https://github.com/KhronosGroup/GLSL/blob/master/extensions/nv/GLSL_NV_ray_tracing.txt
SPV Spec : https://github.com/KhronosGroup/SPIRV-Registry/blob/master/extensions/NV/SPV_NV_ray_tracing.asciidoc

HLSL Info :
https://docs.microsoft.com/en-us/windows/desktop/direct3d12/direct3d-12-raytracing
https://docs.microsoft.com/en-us/windows/desktop/direct3d12/direct3d-12-raytracing-hlsl-reference

AppVeyorBot · 2019-02-12T01:04:15Z

✅ Build DirectXShaderCompiler 1.0.1317 completed (commit 519c89f944 by @alelenv)

dneto0 · 2019-02-12T16:04:32Z

@alelenv : Thanks for this contribution!

For better inclusiveness, I'd prefer to see a copy of the HLSL functional spec in a more plain-text format, for better inclusiveness. Yes the .docx renders apparently correctly on my Mac, but it seems to be straining to do so.

I'm also a little concerned that the reference is on someone's personal .org page, and could disappear at a whim. Ideally a copy would be checked in with this PR (in the docs subtree). However, that doc lacks a copyright or author notice, so ... check with the author?

I don't mean to come off as overly nit-picky, but I'd like this to be a stable documented feature of the compiler.

AppVeyorBot · 2019-02-12T20:21:04Z

✅ Build DirectXShaderCompiler 1.0.1324 completed (commit 307303f3c8 by @alelenv)

ehsannas

Hi @alelenv
Thanks for your contribution. It's great to see this work. The code looks good overall, but needs some improvements.

You'll find that many of my comments are related to adhering to the C++ style guide.

The style guide requires proper Punctuation, Spelling and Grammar. For example, comments should be proper sentences ending with a period. Consistently use SPIR-V, rather than spirv or spir-v or Spir-v, etc in the comments. I know this sounds nit-picky, but it'll lead to a much nicer code base over time (as it has happened over the development period of Spiregg).
clang-format should be applied to all the files that you have touched. (you can use this extension if you're using VisualStudio)
I'm very glad to see you have added new tests \o/ These will prevent regressions in RT code generation in the future. Please add more if you can.
Here are a few that I thought might be useful:
What happens if more than 1 function has the same [shader(X)] attribute? Add a test for it.
Add a test that exercises the matrix transpose logic in processRayBuiltins.
etc.
Taking testing one step further: Would you be able to test the resulting SPIR-V on hardware to ensure correctness? I don't have access to the GPU that has the RT features. That would be the ultimate test for your work.
SPIR-V.rst needs to be updated (we'll discuss this more on the next round of code-review).
Please do not force push to your branch when addressing code-review comments. Please make new commits, so that we can review the diff more easily.
I'll do (at least one) more rounds of code-review and testing before landing.

Thanks!

ehsannas · 2019-02-12T16:27:13Z

tools/clang/include/clang/SPIRV/AstTypeProbe.h

@@ -268,6 +269,11 @@ bool isOrContainsNonFpColMajorMatrix(const ASTContext &,
 /// matrix type.
 QualType getComponentVectorType(const ASTContext &, QualType matrixType);

+/// \brief Return a QualType corresponding to HLSL matrix of given element type


s/Return/Returns

ehsannas · 2019-02-12T16:29:58Z

tools/clang/include/clang/SPIRV/SpirvBuilder.h

@@ -420,6 +420,12 @@ class SpirvBuilder {

  void createLineInfo(SpirvString *file, uint32_t line, uint32_t column);

+  /// \brief Create spirv instructions for NV raytracing ops


Use

/// \brief Creates SPIR-V instructions for NV raytracing ops.

ehsannas · 2019-02-12T16:33:13Z

tools/clang/include/clang/SPIRV/SpirvInstruction.h

@@ -1757,6 +1758,29 @@ class SpirvLineInfo : public SpirvInstruction {
  uint32_t column;
 };

+
+/// \brief Base class for all NV raytracing instructions
+/// These include following spirv opcodes


/// These include the following SPIR-V opcodes:

ehsannas · 2019-02-12T16:38:26Z

tools/clang/lib/SPIRV/AstTypeProbe.cpp

@@ -1012,5 +1016,75 @@ QualType getComponentVectorType(const ASTContext &astContext,
  return astContext.getExtVectorType(elemType, colCount);
 }

+QualType getHLSLMatrixType(ASTContext& astContext, Sema &S,


This is very useful. Thanks for creating the custom matrix QualType 👍

ehsannas · 2019-02-12T16:39:51Z

tools/clang/lib/SPIRV/AstTypeProbe.cpp

+QualType getHLSLMatrixType(ASTContext& astContext, Sema &S,
+                           ClassTemplateDecl *templateDecl,
+                           QualType elemType, int rows, int columns)
+{


the { should be on the previous line.
It looks like you haven't run clang-format ? If not, please do. Thanks.

ehsannas · 2019-02-13T19:24:56Z

tools/clang/lib/SPIRV/ExecutionModel.h

+  ExecutionModel(ShaderKind sk, ExecModel em) : shaderKind(sk), execModel(em) {}
+
+  static const unsigned numExecutionModels = 14;
+  static const ExecutionModel executionModels[numExecutionModels];


In general static member variables are forbidden due to their lifetime and static initialization issues.

static functions are OK.

So, there are a couple of different ways you can address this:

remove these static data members. Have the static getter functions construct an ExecutionModel object and return it. e.g.

static ExecutionModel GetByStageName(llvm::StringRef stageName); static ExecutionModel GetByShaderKind(ShaderKind sk);

there'll be switch statements inside these functions I'd presume.

Another way -- if we really really want to have only 14 objects and reuse them -- would be to use SpirvContext. The context class keeps all the objects that live for the lifetime of the backend. Given how lightweight the objects of this class are, I don't think it's necessary to go down this route.

Can this class be fully replaced with a standalone function (in SpirvEmitter.cpp) that returns the SPIR-V execution model for a given shader model kind?
e.g.

spv::ExecutionModel getSpirvExecutionModel(hlsl::ShaderModel::Kind smk);

ehsannas · 2019-02-13T19:27:46Z

tools/clang/lib/SPIRV/ExecutionModel.h

+  bool IsRay() const {
+    return execModel >= ExecModel::RayGenerationNV &&
+           execModel <= ExecModel::CallableNV;
+  }


All these functions are reimplementing the logic in DxilShaderModel.h.
I wonder if we could just use the hlsl::ShaderModel instead of the ExecutionModel throughout the spiregg code, and remove ExecutionModel class completely? Is there a good reason for this class to exist?

We can't use hlsl::ShaderModel because there are no ShaderModel objects for RT profiles in DxilShaderModel.cpp (rgen_6_4, miss_6_4, etc.).
So I implemented a similar class (ExecutionModel) which would allow me to track individual RT shader stages.
Nonetheless, I have removed ExecutionModel class and used ShaderModel::Kind to track current entry point.
Let me know if this change works for you.

ehsannas · 2019-02-13T19:41:24Z

tools/clang/include/clang/SPIRV/SpirvType.h

@@ -380,6 +381,17 @@ class FunctionType : public SpirvType {
  llvm::SmallVector<const SpirvType *, 8> paramTypes;
 };

+/// Represents accleration structure type as defined in SPV_NV_ray_tracing


SPV_NV_ray_tracing.

ehsannas · 2019-02-13T19:44:42Z

tools/clang/lib/SPIRV/SpirvEmitter.h

+    SpirvFunction *entryFunction;
+    bool isEntryFunction;
+
+    FunctionInfo()


you can remove this constructor.

ehsannas · 2019-02-13T20:52:57Z

tools/clang/lib/SPIRV/SpirvEmitter.cpp

+        if (const auto *shaderAttr = funcDecl->getAttr<HLSLShaderAttr>()) {
+          // If we are compiling as a library then add everything that has a
+          // ShaderAttr
+          FunctionInfo *entryInfo = new FunctionInfo(


As you may have noticed, we don't perform new memory allocation anywhere except in SpirvContext (which uses placement new) which is responsible for managing all allocated memory and releasing them.

I suggest you change your data structure from

llvm::DenseMap<const DeclaratorDecl *, const FunctionInfo *> functionInfoMap;

to

llvm::DenseMap<const DeclaratorDecl *, FunctionInfo> functionInfoMap;

and then

functionInfoMap[funcDecl] = FunctionInfo(...); workQueue.insert(&functionInfoMap[funcDecl]);

... hmmm... it looks like you can further simplify this:

Remove functionInfoMap and change

llvm::SetVector<const DeclaratorDecl *> workQueue;

to

llvm::MapVector<const DeclaratorDecl *, FunctionInfo> workQueue;

this way, workQueue[funcDecl] is the same as what funcMapInfo[funcDecl] used to be ; and if I understand llvm::MapVector correctly, you can iterate over it the same as as SetVector, so you can still have the loop over workQueue.

@sparmarNV
I'm doing further testing on this PR, and I'm running into some failures. They are due to this suggestion that I made:

functionInfoMap[funcDecl] = FunctionInfo(...); workQueue.insert(&functionInfoMap[funcDecl]);

It turns out llvm::DenseMap may choose to overwrite that address (&functionInfoMap[x]) when handling its buckets. Sorry about that. I have made the fix, and I'll post my fix to this PR directly. I have created a wrapper function that updates functionInfoMap and workQueue at the same time, and it uses placement new to allocate memory from the SpirvContext memory pool.

Thanks for testing our changes and I am glad that you were able to triage and revert this change before the merge.

alelenv · 2019-02-13T21:49:53Z

@dneto0 : Sure. We are working on providing you a text copy soon which we can check into docs.

@ehsannas : Thanks for such a detailed review.
Some quick answers below. sparmarNV will be handling issues related to multiple entry points support.
We will update branch based on your feedback soon.

Q) Add a test that exercises the matrix transpose logic in processRayBuiltins. etc.
ObjectToWorld3x4() & WorldToObject3x4() are transposes of SPIR-V Builtin
A) This is already tested in raytracing.nv.closesthit.hlsl/raytracing.nv.anyhit.hlsl

Q) Taking testing one step further: Would you be able to test the resulting SPIR-V on hardware to ensure correctness? I don't have access to the GPU that has the RT features. That would be the ultimate test for your work.
A) We have some internal apps running on HW using the generated SPIR-V.

AppVeyorBot · 2019-02-14T01:32:35Z

✅ Build DirectXShaderCompiler 1.0.1331 completed (commit 6e257b7d22 by @alelenv)

ehsannas · 2019-02-14T20:37:57Z

A) This is already tested in raytracing.nv.closesthit.hlsl/raytracing.nv.anyhit.hlsl

Cool! I couldn't find OpTranspose in your CHECK: commands in the tests. So it would be good to add a CHECK. This will firstly ensure OpTranspose is actually called (and not accidentally removed in the future), and it'll also be good to have a check to see the custom matrix type (e.g. 4x3 or 3x4) as the result type in a CHECK.

A) We have some internal apps running on HW using the generated SPIR-V.

Awesome! Glad to hear that.

Update tests to add multple entry functions of same shader stage

alelenv · 2019-02-19T20:33:00Z

HLSL Spec is provided by Microsoft to users registered to DX forums
Please follow instructions provided below to register
http://forums.directxtech.com/index.php?topic=5985.0

I have updated link above to publicly available HLSL specs from MSDN

AppVeyorBot · 2019-02-19T21:22:35Z

✅ Build DirectXShaderCompiler 1.0.1352 completed (commit 91ed87a3f5 by @alelenv)

- This change removes ExecutionModel class and relies on ShaderModel::Kind to track current entry point shader stage - Also instead of declaring it in SpirvEmitter, DeclResultIdMapper & GlPerVertex, we declare it only once in common object SpirvContext

AppVeyorBot · 2019-02-20T00:43:38Z

✅ Build DirectXShaderCompiler 1.0.1354 completed (commit 282658bec5 by @sparmarNV)

ehsannas

@sparmarNV

Thanks for making this change 👍 . This is much nicer than what was there before.
You have eliminated a whole class, eliminated the use of static members, eliminated copies of ExecutionModel in different places and concentrated it all in SpirvContext. All great changes 👍 .

Please run clang-format if you haven't already.

ehsannas · 2019-02-20T20:46:53Z

tools/clang/lib/SPIRV/SpirvEmitter.cpp

-    declIdMapper.setSpvExecutionModel(curEntryOrCallee->spvExecModel);
-    spvExecModel = curEntryOrCallee->spvExecModel;
-    entryFunction = curEntryOrCallee->entryFunction;
+    spvContext.setCurrentShaderModelKind(curEntryOrCallee->shaderModelKind);


Great to see the 3 lines is replaced with 1 line. Shader kind is now only stored in 1 place (SpirvContext), and this makes the code less error-prone. 👍

ehsannas · 2019-02-20T20:51:10Z

tools/clang/include/clang/SPIRV/SpirvContext.h

+  // Current ShaderModelKind for entry point.
+  ShaderModelKind curShaderModelKind;
+  // Major/Minor hlsl profile version.
+  unsigned majorVersion;


use uint32_t instead of unsigned to be consistent with the rest of the code.

@sparmarNV : Could you comment on this?

I see both unsigned and uint32_t being used in the file.
Nonetheless, I will update the type to uint32_t.

Looks like a few unsigned have snuck through in the past. But let's strive to stay consistent. I'll update the other ones to also use uint32_t in a different PR. Thanks for updating.

ehsannas · 2019-02-20T20:52:52Z

tools/clang/lib/SPIRV/DeclResultIdMapper.h

@@ -366,12 +362,6 @@ class DeclResultIdMapper {
  /// \brief Sets the entry function.
  void setEntryFunction(SpirvFunction *fn) { entryFunction = fn; }

-  /// \brief Sets the SPIR-V execution model


Happy to see this is gone.

ehsannas · 2019-02-20T20:53:18Z

tools/clang/lib/SPIRV/DeclResultIdMapper.h

@@ -648,15 +638,13 @@ class DeclResultIdMapper {
  inline bool isInputStorageClass(const StageVar &v);

 private:
-  const hlsl::ShaderModel &shaderModel;


Happy to see this is gone.

ehsannas · 2019-02-20T20:53:38Z

tools/clang/lib/SPIRV/DeclResultIdMapper.h

  SpirvBuilder &spvBuilder;
  SpirvEmitter &theEmitter;
  const SpirvCodeGenOptions &spirvOptions;
  ASTContext &astContext;
  SpirvContext &spvContext;
  DiagnosticsEngine &diags;
  SpirvFunction *entryFunction;
-  const ExecutionModel *spvExecModel;


Happy to see this is gone.

ehsannas · 2019-02-20T20:58:48Z

tools/clang/lib/SPIRV/SpirvEmitter.cpp

-              ExecutionModel::GetByShaderKind(shaderModel.GetKind()), funcDecl,
-              /*entryFunction*/ nullptr, /*isEntryFunc*/ true);
+          FunctionInfo *entryInfo =
+              new FunctionInfo(spvContext.getCurrentShaderModelKind(), funcDecl,


I believe new should be avoided here. See my comment about workQueue

Yes, I am working on this next. Thanks for the review 👍

Cool. Thanks 👍

I tried replacing functionInfoMap and workQueue with a MapVector, but it seems MapVector has few limitations.
We can't access the vector elements inside the MapVector with an index, which doesn't work in below case -

// The queue can grow in the meanwhile; so need to keep evaluating
// workQueue.size().
for (uint32_t i = 0; i < workQueue.size(); ++i) {
const FunctionInfo *curEntryOrCallee = workQueue[i];
...
}

Also using iterator on MapVector doesn't work with growing queue.
So I have gone with your option 1, which is to remove 'new' memory allocation by changing the DenseMap decl to -

llvm::DenseMap<const DeclaratorDecl *, FunctionInfo> functionInfoMap;

@sparmarNV Ah you're right. It is not possible to iterate over a MapVector when it is growing. Thanks for removing the usage of new.

ehsannas · 2019-02-20T20:58:59Z

tools/clang/lib/SPIRV/SpirvEmitter.cpp

@@ -613,14 +618,19 @@ void SpirvEmitter::HandleTranslationUnit(ASTContext &context) {
  spvBuilder.setMemoryModel(spv::AddressingModel::Logical,
                            spv::MemoryModel::GLSL450);

+  // Even though the 'workQueue' grows due to the above loop, the first
+  // 'numEntryPoints' entries in the 'workQueue' are the ones with the HLSL
+  // 'shader' attribute, and must therefore be entry functions.


Thanks for adding.

ehsannas · 2019-02-20T20:59:45Z

tools/clang/lib/SPIRV/SpirvEmitter.cpp

-    FunctionInfo *calleeInfo = new FunctionInfo(
-        spvExecModel, callee, /*entryFunction*/ nullptr, /*isEntryFunc*/ false);
+    FunctionInfo *calleeInfo =
+        new FunctionInfo(spvContext.getCurrentShaderModelKind(), callee,


I believe new should be avoided here. See my comment about workQueue

ehsannas · 2019-02-20T21:08:34Z

tools/clang/lib/SPIRV/SpirvEmitter.h

@@ -593,6 +592,11 @@ class SpirvEmitter : public ASTConsumer {
  /// Emits an error if the given attribute is not a loop attribute.
  spv::LoopControlMask translateLoopAttribute(const Stmt *, const Attr &);

+  static hlsl::ShaderModel::Kind
+  SpirvEmitter::getShaderModelKind(StringRef stageName);


No need for SpirvEmitter::

static hlsl::ShaderModel::Kind getShaderModelKind(StringRef stageName);

This is caught by buildbots on Linux and macOS
Same for getSpirvShaderStage below.

ehsannas · 2019-02-20T21:12:44Z

tools/clang/lib/SPIRV/ExecutionModel.h

@@ -1,62 +0,0 @@
-//===------- ExecutionModel.h - get/set SPV Execution Model -----*- C++ -*-===//


Happy to see that the class is gone. Especially the static data that was included in it.

AppVeyorBot · 2019-02-21T19:58:00Z

✅ Build DirectXShaderCompiler 1.0.1390 completed (commit e03243ece5 by @alelenv)

This change also - - removes invalid "SpirvEmitter::" from function declarations in SpirvEmitter class. - fix build errors by adding a default constructor in FunctionInfo struct to allow functionInfoMap allocate an empty object for no search results.

AppVeyorBot · 2019-02-21T22:34:37Z

✅ Build DirectXShaderCompiler 1.0.1392 completed (commit d91e766eae by @sparmarNV)

alelenv · 2019-02-22T00:28:51Z

@ehsannas : I believe we have completed updates based on your feedback for first round of reviews
Can you point us to next steps i.e how to update SPIR-V.rst?
Thanks

ehsannas

@alelenv Thanks for addressing my comments so far. If you go through all the files, there are a few (mostly formatting) ones left to fix.

We're almost there.

Regarding documentation:
We have a comprehensive doc file about mapping HLSL concepts to SPIR-V. The file (SPIR-V.rst) is located here. You should update this file to reflect the changes you have made in this PR.

I think it'd be good to add a top-level section about Ray Tracing between Shader Model 6.0 Wave Intrinsics and Supported Command-line Options sections. And in that Ray Tracing section, feel free to add as many sub-sections as you need. The goal is to highlight the the RTX concepts/features and how they map from HLSL to SPIR-V. In other sections of this document we've tried to provide simple explanations for people to get started, and provide external links for full details. For example, you can also provide a link to the RTX HLSL spec, and the SPIR-V NV RT extension spec. It'd also be really nice to provide a link to sample shaders (e.g. raytracing helloworld shaders) that people can use to get started.

Under HLSL Shader Stages, you should add a Ray Tracing Stages subsection, and add the newly supported shader stages under that. For each stage, it'd be good to provide some explanation of the stage and also list the expected shader inputs and outputs (stage variables). Including Figure 1 and Figure 5 from @nsubtil's blog post could be very useful here.

In general I found that blog post to be quite a good introduction. You could use some of its content as brief explanations for the new documentation you are adding to SPIR-V.rst.

Thanks!

ehsannas · 2019-02-22T15:23:29Z

tools/clang/lib/SPIRV/SpirvFunction.cpp

@@ -34,7 +34,8 @@ bool SpirvFunction::invokeVisitor(Visitor *visitor, bool reverseOrder) {
  if (!basicBlocks.empty()) {
    BlockReadableOrderVisitor([&orderedBlocks](SpirvBasicBlock *block) {
      orderedBlocks.push_back(block);
-    }).visit(basicBlocks.front());
+    })


revert to previous format (clang-format).

ehsannas · 2019-02-22T15:23:45Z

tools/clang/lib/SPIRV/SpirvEmitter.h

@@ -1046,6 +1084,16 @@ class SpirvEmitter : public ASTConsumer {
  /// Maps a given statement to the basic block that is associated with it.
  llvm::DenseMap<const Stmt *, SpirvBasicBlock *> stmtBasicBlock;

+  /// Maintains mapping from a type to spirv variable along with spirv


s/spirv/SPIR-V

ehsannas · 2019-02-22T15:28:15Z

tools/clang/include/clang/SPIRV/SpirvBuilder.h

@@ -420,6 +420,12 @@ class SpirvBuilder {

  void createLineInfo(SpirvString *file, uint32_t line, uint32_t column);

+  /// \brief Creates spirv instructions for NV raytracing ops


s/spirv/SPIR-V.
ops.

ehsannas · 2019-02-22T15:30:40Z

tools/clang/lib/SPIRV/SpirvEmitter.cpp

+  SpirvInstruction *retVal =
+    declIdMapper.getBuiltinVar(builtin, builtinType, callExpr->getExprLoc());
+  retVal = spvBuilder.createLoad(builtinType, retVal);
+  if (transposeMatrix)


This is done now. Thanks.

ehsannas · 2019-02-22T15:32:03Z

tools/clang/lib/SPIRV/SpirvEmitter.cpp

+      spvBuilder.createLoad(hitAttributeArg->getType(), hitAttributeArgInst);
+  spvBuilder.createStore(hitAttributeStageVar, tempLoad);
+
+  // SPV Instruction :


s/SPV/SPIR-V

ehsannas · 2019-02-22T15:38:36Z

tools/clang/lib/SPIRV/SpirvEmitter.cpp

+    builtin = spv::BuiltIn::WorldToObjectNV;
+    break;
+  default:
+    emitError("ray intrinsic function unimplemented", callExpr->getExprLoc());


Should we return nullptr after emitError?

ehsannas · 2019-02-22T15:40:53Z

tools/clang/lib/SPIRV/SpirvEmitter.cpp

+      spvBuilder.createLoad(callDataArg->getType(), callDataArgInst);
+  spvBuilder.createStore(callDataStageVar, tempLoad);
+
+  // SPV Instruction


s/SPV/SPIR-V

AppVeyorBot · 2019-02-25T04:11:27Z

✅ Build DirectXShaderCompiler 1.0.1400 completed (commit 2d8ad29f11 by @alelenv)

AppVeyorBot · 2019-02-25T05:35:07Z

✅ Build DirectXShaderCompiler 1.0.1401 completed (commit 76ae56e1e5 by @alelenv)

AppVeyorBot · 2019-02-25T08:30:12Z

✅ Build DirectXShaderCompiler 1.0.1402 completed (commit 14e6d17c58 by @sparmarNV)

ehsannas · 2019-02-25T14:50:00Z

@alelenv @sparmarNV Thanks for your updates.
Sorry I forgot to mention one thing about the doc file:
There's a Intrinsic functions section in the doc file. Please add a subsection for Ray Tracing Intrinsics, and put in a table (example) to show the HLSL-->SPIR-V mapping for the new intrinsics that you have added support for.
Thanks!

AppVeyorBot · 2019-02-25T19:48:46Z

✅ Build DirectXShaderCompiler 1.0.1403 completed (commit a5ead487d5 by @alelenv)

Also bundle the insertion into functionInfoMap and workQueue together.

alelenv · 2019-02-25T22:07:04Z

@ehsannas : Sorry for missing a large number of formatting fixes from previous comments. My tracking changes for each comment via emails didnt work out properly.
I have updated rst with raytracing info, mappings and flowchart diagrams
Let me know your thoughts

Thanks!

AppVeyorBot · 2019-02-25T23:45:12Z

✅ Build DirectXShaderCompiler 1.0.1413 completed (commit a0c65a70d6 by @alelenv)

ehsannas · 2019-02-26T15:40:24Z

Just finished reading the documentation. Awesome job, @alelenv 👍. This is exactly what we were looking for.
It provides a good overview and contains links for readers to go off and learn more (specs and tutorials).

ehsannas

LGTM. Thanks for addressing all the code review comments \o/
I will do a bit of more testing, and if all goes well, I'll merge the PR (hopefully by tomorrow).

AppVeyorBot · 2019-02-26T17:39:15Z

✅ Build DirectXShaderCompiler 1.0.1416 completed (commit ee8f446f5a by @ehsannas)

AppVeyorBot · 2019-02-26T19:56:06Z

✅ Build DirectXShaderCompiler 1.0.1420 completed (commit 941395b8de by @alelenv)

ehsannas self-requested a review February 12, 2019 15:52

ehsannas added the spirv Work related to SPIR-V label Feb 12, 2019

alelenv force-pushed the NV_ray_tracing_final branch from 5c5b9a7 to 549193c Compare February 12, 2019 18:15

Add support to convert DXR HLSL to SPV_NV_ray_tracing

b5dcf5c

alelenv force-pushed the NV_ray_tracing_final branch from b9bb79b to b5dcf5c Compare February 12, 2019 19:20

ehsannas suggested changes Feb 13, 2019

View reviewed changes

Fix multiple typos and cleanup using clang-format.

2cc4af4

Update tests to verify transpose and custom matrix type

b91b791

Update tests to add multple entry functions of same shader stage

ehsannas suggested changes Feb 20, 2019

View reviewed changes

Dont create a stageVar for raytracing interface variables.

082ef0d

ehsannas reviewed Feb 22, 2019

View reviewed changes

Fix some more typos and fomatting errors.

3ff89ba

Update RST with raytracing stage info

3524456

In SpirvContext.h, replace unsigned by uint32_t

988cae6

Test add ascii art and fixup grammar mistakes.

6739366

ehsannas and others added 4 commits February 25, 2019 15:33

Use placement new to allocate FunctionInfo objects.

9ec5b40

Also bundle the insertion into functionInfoMap and workQueue together.

Remove outdated comment.

aa5d0e5

Update RST with intrinsic mapping and typo fixes.

02a8044

Some more wording fixes to RST for raytracing.

5028dfb

alelenv added 2 commits February 25, 2019 14:17

Accidently broke table in RST due to missing '-'

2ec45df

Add tick marks for supported stages in RST

b91c417

Final clang-format formatting fixes.

fd5243b

ehsannas approved these changes Feb 26, 2019

View reviewed changes

Add missing labels to flowchart and spacing.

856f601

ehsannas merged commit e58ecd4 into microsoft:master Feb 27, 2019

ehsannas mentioned this pull request Feb 27, 2019

[spirv] Request for support for DXR to VK_NV_raytracing! #1554

Closed

Jasper-Bekkers mentioned this pull request Mar 11, 2019

[SPIRV] Emit ShaderRecordBufferNV in ray hit shader #2019

Closed

		@@ -420,6 +420,12 @@ class SpirvBuilder {

		void createLineInfo(SpirvString *file, uint32_t line, uint32_t column);

		/// \brief Create spirv instructions for NV raytracing ops

		@@ -1,62 +0,0 @@
		//===------- ExecutionModel.h - get/set SPV Execution Model ------ C++ --===//

		@@ -420,6 +420,12 @@ class SpirvBuilder {

		void createLineInfo(SpirvString *file, uint32_t line, uint32_t column);

		/// \brief Creates spirv instructions for NV raytracing ops

Add support to convert DXR HLSL to SPV_NV_ray_tracing #1920

Add support to convert DXR HLSL to SPV_NV_ray_tracing #1920

Conversation

alelenv commented Feb 11, 2019

msftclas commented Feb 11, 2019 • edited Loading

alelenv commented Feb 12, 2019 • edited Loading

AppVeyorBot commented Feb 12, 2019

dneto0 commented Feb 12, 2019

AppVeyorBot commented Feb 12, 2019

ehsannas left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sparmarNV Feb 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alelenv commented Feb 13, 2019 • edited Loading

AppVeyorBot commented Feb 14, 2019

ehsannas commented Feb 14, 2019

alelenv commented Feb 19, 2019

AppVeyorBot commented Feb 19, 2019

AppVeyorBot commented Feb 20, 2019

ehsannas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AppVeyorBot commented Feb 21, 2019

AppVeyorBot commented Feb 21, 2019

alelenv commented Feb 22, 2019

ehsannas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AppVeyorBot commented Feb 25, 2019

AppVeyorBot commented Feb 25, 2019

AppVeyorBot commented Feb 25, 2019

ehsannas commented Feb 25, 2019

AppVeyorBot commented Feb 25, 2019

alelenv commented Feb 25, 2019

AppVeyorBot commented Feb 25, 2019

ehsannas commented Feb 26, 2019

ehsannas left a comment

Choose a reason for hiding this comment

AppVeyorBot commented Feb 26, 2019

AppVeyorBot commented Feb 26, 2019

msftclas commented Feb 11, 2019 •

edited

Loading

alelenv commented Feb 12, 2019 •

edited

Loading

ehsannas left a comment •

edited

Loading

sparmarNV Feb 19, 2019 •

edited

Loading

alelenv commented Feb 13, 2019 •

edited

Loading