aaron.ballman accepted this revision.
aaron.ballman added a comment.
LGTM!
================
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.
----------------
beanz wrote:
> 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.
(I need to shower after learning this cursed knowledge, and I fear I will never
be clean again.)
Thank you for the discussion on this (both here and on IRC). It sounds like we
could get away with using a declaration attribute and then punt all the
complexity back to the backend which handles it entirely from the
initialization of the marked-precise variable. Not ideal, but far better than
trying to model this in the frontend.
================
Comment at: clang/docs/HLSLSupport.rst:197
+* Any use of the ``virtual`` keyword
+* Most features C++11 and later
----------------
beanz wrote:
> 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
Excellent!
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
