[PATCH] D123278: [Clang] [Docs] Add HLSLSupport page

Thu Apr 7 14:19:48 PDT 2022

beanz added a comment.

I'll do an edit pass tonight and update. I've also gone through and replied to some of the questions and comments inline.

Thank you!

================
Comment at: clang/docs/HLSLSupport.rst:22-23
+<https://github.com/microsoft/DirectXShaderCompiler/>`_. in all its supported
+use cases. Accomplishing that goal will require a high level of source
+compatibility.
+
----------------
kuhar wrote:
> What do you mean by source compatibility in this case? In terms of the input source language (HLSL), or the source code of both implementations?
> 
> Perhaps we could make it more direct? E.g., 'This requires us to add the HLSL language support to Clang.'
I see the confusion there. I mean in terms of the input source, not the implementation. We do vend the compiler as a library too with a COM API to invoke the compiler on in-memory sources (which is slightly terrifying to me).

I don't yet know how much flexibility we have to radically change the API, but that shouldn't have any impact on the implementations in clang because we'll do it in a wrapper library.

================
Comment at: clang/docs/HLSLSupport.rst:50-52
+HLSL is not a formally or fully specified language, and while our goals require
+a high level of source compatibility, implementations can vary and we have some
+flexibility to be more or less permissive in some cases.
----------------
kuhar wrote:
> Is DXC the reference implementation?
Yes, with the caveat that it doesn't actually support older language versions (for that we have FXC).

================
Comment at: clang/docs/HLSLSupport.rst:73
+Following the examples of other parser extensions HLSL will add a ParseHLSL.cpp
+file to contain the implementations of HLSL-specific extensions to the clang
+parser.
----------------
aaron.ballman wrote:
> Probably also worth pointing out that most HLSL parsing is expected to be the usual C and C++ code paths, or is that not your expectation?
Yea, that is the expectation.

================
Comment at: clang/docs/HLSLSupport.rst:90-91
+metadata. *hand wave* *hand wave* *hand wave* As a design principle here we want
+our IR to be idiomatic Clang IR as much as possible. We will use IR attributes
+wherever we can, and use metadata as sparingly as possible. One example of a
+difference from DXC already implemented in Clang is the use of target triples to
----------------
kuhar wrote:
> Are all attributes guaranteed to be preserved? I thought some might get dropped by opt.
Some attributes do get dropped in optimization. We are going to need to evaluate a bit case by case. DXC (and DXIL) puts a lot of information in metadata, which is a pain to work with through compiler layers. We actually have a set of helpers to read data in and out of the IR metadata... I'm trying to avoid needing any of that.

================
Comment at: clang/docs/HLSLSupport.rst:121-122
+operators (unary ``*``, and ``->``), as well as the address of operator (unary
+&). While HLSL disallows pointers and references in the syntax, HLSL does use
+reference types in the AST.
+
----------------
aaron.ballman wrote:
> Presumably it also uses pointer types in the AST for things like array and function decay?
That is my plan for in Clang. In DXC they prevent array->pointer decay in the AST which IMO causes more problems than it solves, especially since the IR decays to pointers anyways.

================
Comment at: clang/docs/HLSLSupport.rst:143-145
+In HLSL 2018 and earlier, HLSL supported logical operators (and the ternary
+operator) on vector types. This behavior required that operators not short
+circuit.
----------------
aaron.ballman wrote:
> It's not clear whether the behavior will vary for all types or just vector types. Also, does this apply in preprocessor conditionals the same as runtime expressions?
It only applies in runtime expressions, and does apply to all types not just vectors. Vectors are the motivation for it because short circuiting would be unwieldy for vector types.

With HLSL 2021, operators follow C short circuit rules and are not allowed to operate on vector types, instead there are builtin functions to handle the vector cases.

================
Comment at: clang/docs/HLSLSupport.rst:150-152
+HLSL has a ``precise`` qualifier that behaves unlike anything else in the C
+language. The support for this qualifier in DXC is buggy, so our bar for
+compatibility is low.
----------------
aaron.ballman wrote:
> Is it worth supporting at all (I know you want source compatibility...)? Type system changes are generally expensive and invasive, largely because changing the type system in C++ is complicated. For example, does this qualifier impact overload sets or template specializations? Does it get name mangled? That sort of thing.
Unfortunately we do need to implement it to some degree. In DXC it is implemented as an attribute rather than a qualifier, so it shouldn't impact overloads, template specialization or mangling.

I call it a qualifier here because it is syntactically more like a qualifier, but I'm honestly not sure how we should implement it. In DXC, we emit IR metadata denoting the underlying value as "precise", then in an IR pass propagate disabling fast math.

================
Comment at: clang/docs/HLSLSupport.rst:185
+* ``union`` types `(in progress for HLSL 202x) <https://github.com/microsoft/DirectXShaderCompiler/pull/4132>`_
+* Most features C11 and later
+
----------------
aaron.ballman wrote:
> From C99:
> VLAs?
> _Complex/_Imaginary?
> 
> From C11:
> Threads/Atomics?
> Language features like `_Generic` (or `[[]]` attributes in C2x)?
> 
> Also, it's not clear whether you expect these unsupported features to be diagnosed or supported as extensions in HLSL, etc.
Yea... none of those are supported either.

C11 atomics might actually be interesting because I _think_ you could make them work.

There's also probably no good reason why `_Generic` wouldn't work.

My plan is to bring in `[[]]` attributes in HLSL 202x, so those get to stay :).

================
Comment at: clang/docs/HLSLSupport.rst:187
+
+HLSL does not support the following C++ features:
+
----------------
kuhar wrote:
> These are C++ language features. I assume that all library features are also out of the window?
Yep, I was focused on language, but should mention the library.

================
Comment at: clang/docs/HLSLSupport.rst:189
+
+* RTTI
+* Exceptions
----------------
aaron.ballman wrote:
> No `dynamic_cast` or `typeid`?
Yep, no C++ casting at all (although we've discussed adding static and reinterpret casts).

================
Comment at: clang/docs/HLSLSupport.rst:190
+* RTTI
+* Exceptions
+* Multiple inheritance
----------------
kuhar wrote:
> How about goto and labels? Irreducible control flow? Are infinite loops valid?
No support for goto or labels. I think infinite loops are undefined behavior, but our resident HLSL language expert is on vacation and I can't find documentation.

================
Comment at: clang/docs/HLSLSupport.rst:193
+* Access specifiers
+* Anonymous namespaces
+* ``new`` & ``delete`` operators
----------------
aaron.ballman wrote:
> Inline namespaces?
Not supported, also not a feature I was aware of in C++... Learn something new every day...

================
Comment at: clang/docs/HLSLSupport.rst:194
+* Anonymous namespaces
+* ``new`` & ``delete`` operators
+* Constructors & destructors
----------------
aaron.ballman wrote:
> I presume...
> 
> How about placement new expressions?
Also not supported.

================
Comment at: clang/docs/HLSLSupport.rst:197
+* Any use of the ``virtual`` keyword
+* Most features C++11 and later
----------------
aaron.ballman wrote:
> Same question here as above with C on whether we expect to support those as extensions or diagnose them as invalid.
I didn't really answer that above. I am inclined to do a mix of both. Anything that we can support and lower I'd like to treat as extensions. Some things we just can't.

We don't currently have any way to allocate memory dynamically, so there's really no way to support new/delete. RTTI and exceptions are another case where we just don't have the support we would need in the driver specifications.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123278