beanz added inline comments.
================
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:
> beanz wrote:
> > aaron.ballman wrote:
> > > beanz wrote:
> > > > 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.
> > > > 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.
> > >
> > > Okay, I kind of figured you needed it.
> > >
> > > > 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.
> > >
> > > This smells more like an attribute than a qualifier to me, but I'd like
> > > to see if I understand first (please ignore if I get syntax wrong):
> > > ```
> > > // Initialization
> > > precise float f = 1.0 / 3.0 * sinf(3.14f); // value computation is precise
> > > // Assignment
> > > f = 1.0 / 3.0 * sinf(3.14f); // value computation is NOT precise (not a
> > > declaration)?
> > > // RHS of expression
> > > float f2 = f * 2.0; // value computation is NOT precise (f2 is not marked
> > > precise)?
> > > ```
> > >
> > Yea, your syntax and behavior is correct with the added complication that
> > precise upward propagates. The upward propagation is the part that is wonky
> > and problematic.
> >
> > ```
> > float f = 1.0 * sinf(3.14f); // precise
> > precise float f2 = f / 2.0f; // precise
> > float f3 = f * f2; // not precise
> > ```
> >
> Sorry if I'm being dense, but I'm still lost. Why is `f` precise in your
> example?
>
> But more interestingly, is this a property of the *initialization* of the
> variable of that type only, or is it a property of the type itself where
> arbitrary expressions involving that type need to care whether it's precise
> or not?
>
> If it's just about the initialization, then I think a declaration attribute
> is the clear winner for the semantic support. If it needs to be tracked
> through arbitrary expressions involving the type, it's probably better as a
> qualifier sort of like the nullability ones. e.g.,
> ```
> template <typename Ty>
> void func(Ty Val) {
> Val += 1.0f // Does this get precise semantics?
> }
>
> int main() {
> precise float f = 1.0f;
> func(f);
> }
> ```
>
>
`precise` behaves unlike anything I've ever encountered in a programming
language before. It neither follows the type nor the declared value.
`precise` never propagates down (so a use of a `precise` value does not make
subsequent things `precise`. It does propagate up to values that contribute to
the `precise`, which is just... weird.
I don't know whether HLSL or GLSL (or some other SL) did it first, but GLSL has
a spec which states:
> The **precise** qualifier ensures that operations contributing to a
> variable's value are done in their stated order and with operator
> consistency.
The GLSL spec expands into a whole lot more details and examples:
https://www.khronos.org/registry/OpenGL/specs/gl/GLSLangSpec.4.60.pdf
HLSL's behavior //should// match GLSL (and some other vendor-specific
languages), but frequently doesn't.
================
Comment at: clang/docs/HLSLSupport.rst:197
+* Any use of the ``virtual`` keyword
+* Most features C++11 and later
----------------
aaron.ballman wrote:
> beanz wrote:
> > aaron.ballman wrote:
> > > beanz wrote:
> > > > 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.
> > > Okay, that seems reasonable enough. Should we issue "this is a Clang
> > > extension" diagnostics in the places where you're supporting something as
> > > an extension? Also, I presume you intend to add error diagnostics for all
> > > the cases that are not supported?
> > I think issuing clang extension diagnostics will be super valuable to our
> > users. My expectation is that it will take a few years to transition all of
> > our users from DXC to Clang, and in the meantime if they are supporting
> > both compilers in their codebases diagnostics for extension use will be
> > immensely helpful.
> >
> > For unsupported features, I think the immovable priority is that we can't
> > succeed compiling and generate invalid code (in our primary case that would
> > be code that can't be run under one of our driver specifications).
> >
> > Having nice, targeted diagnostics for each unsupported case is the ideal,
> > but there are cases (like with Microsoft attribute parsing on templates),
> > where we fail the compile with diagnostics that are "good enough" or even
> > just in parity to DXC which we may take as an opportunity for future
> > improvement and not a requirement.
> >
> > Does that make sense? I'm trying not to back myself into a corner of having
> > perfect be the enemy of "better than what we have today".
> > I think issuing clang extension diagnostics will be super valuable to our
> > users.
>
> +1, I think it'd be useful as well, for exactly that situation.
>
> > For unsupported features, I think the immovable priority is that we can't
> > succeed compiling and generate invalid code (in our primary case that would
> > be code that can't be run under one of our driver specifications).
>
> +1, definitely agreed.
>
> > Having nice, targeted diagnostics for each unsupported case is the ideal,
> > but there are cases (like with Microsoft attribute parsing on templates),
> > where we fail the compile with diagnostics that are "good enough" or even
> > just in parity to DXC which we may take as an opportunity for future
> > improvement and not a requirement.
> >
> > Does that make sense? I'm trying not to back myself into a corner of having
> > perfect be the enemy of "better than what we have today".
>
> Yes, this makes sense. Mostly, I want to ensure that code which is invalid
> causes errors in the frontend rather than errors in the backend or at runtime
> when it comes to unsupported features. So it's not so much about what
> diagnostics are generated as it is about ensuring diagnostics are generated
> where necessary.
I think You and I are 100% on the same page here. Fail early with actionable
errors, rather than asserting in the backend ;P
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D123278/new/
https://reviews.llvm.org/D123278
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
