Re: [WGSL] Test for round-trippability

[This conversation has migrated to https://github.com/gpuweb/gpuweb/issues/692 <https://github.com/gpuweb/gpuweb/issues/692>]

> On Apr 2, 2020, at 6:31 AM, Dan Sinclair <dsinclair@google.com> wrote:
> 
> I believe some of the issues pointed out can be resolved before the round tripability has been resolved. I've already commented on the `for` loop issue. For others, please tag them with the for meeting label and we can discuss them at the next meeting.
> 
> Thanks,
> dan
> 
> 
> On Wed, Apr 1, 2020 at 2:24 PM Maciej Stachowiak <mjs@apple.com <mailto:mjs@apple.com>> wrote:
> 
> 
>> On Apr 1, 2020, at 10:07 AM, Dan Sinclair <dsinclair@google.com <mailto:dsinclair@google.com>> wrote:
>> 
>> Responding to the points raised:
>> 1- This seems to be referring to the for loop question? As we’ve stated in the past, adding for loops is something we’re fine with, we can desugar then to loop/continuing. If this is referring to other issues can you please reference them for me.
> 
> For loops, while loops, comma, regardless, unless, comma operator, lexical scope vs dominator scope are some of the ones I can think of that would be potentially affected by this.
> 
> For for loops, there are multiple sub-issues:
> - Some have argued they should not be added to the language at all, even as a desugaring.
> - There’s specifically been debate over for loops with multiple statements in the iterator position (either via comma operator or a block statement; but even without these things it seems like if (true) should be a workaround)
> - There’s been much debate over whether a powerful enough for could replace loop/continuing entirely.
> 
> My point is not to rehash all these issues in this email thread. Just that we need specifics of what distinctions must be preserved on a SPIR-V —> WGSL —> SPIR-V round trip, and some verifiable evidence for why those distinctions must be preserved.
> 
>> 
>> 2- This is in flux with the outstanding FAQ changes and the outstanding goals section changes.
> 
> We need details on what exactly needs to be preserved on a round trip and why (and some evidence if the specifics seem mysterious), in order to evaluate the planned FAQ and goals section changes.
> 
> 
>> 3- Chrome and Firefox both have to be performant on MSL and HLSL based platforms. We are working to support all of those along with SPIR-V as we have to run on all those platforms. Can you please file issues for constructs in WGSL which will not be performant on MSL?
> 
> We don’t think there are any, but since Google has reported the developer feedback on needing to fine-tune SPIR-V, perhaps you have heard of issues with other shader languages. We’re also asking the Metal team at Apple.
> 
>> 
>> We have outstanding tasks to get the FAQ in place and to update the Goals section. David has an outstanding task to writeup the initial round tripping documentation. We think this conversation should wait on those tasks to get completed first.
> 
> It’s fine to wait, we’d just like an ETA. Some of the issues above seem stalled on having this info.
> 
> Maybe some can be cleanly resolved without it. If we’d like to try that exercise, we could provide links to relevant GitHub Issues.
> 
> 
>> 
>> Thanks,
>> Dan
>> 
>> 
>> On Tue, Mar 31, 2020 at 3:12 PM Maciej Stachowiak <mjs@apple.com <mailto:mjs@apple.com>> wrote:
>> 
>> 
>>> On Mar 31, 2020, at 9:25 AM, David Neto <dneto@google.com <mailto:dneto@google.com>> wrote:
>>> 
>>> Myles,
>>> 
>>> Thanks for trying to advance the conversation.
>>> 
>>> Google suggests that the CG defer defining the round-trip requirements until we have had the time and opportunity to publish further updates to the WGSL specification. The current WGSL snapshot is known to be incomplete in many areas, and was hastened toward publication in order to enable forward progress in the CG.
>>> 
>>> Based on feedback from developers with whom we collaborate, the definition of WGSL in terms of SPIR-V semantics, and round-trippability to SPIR-V, are hard requirements. The criteria Apple proposes seem to allow significant deviation from these requirements. Still, we would like to avoid responding point-by-point at the present time, in the hope that further clarifications to the current WGSL specification may alleviate concerns and make it easier to reach consensus.
>>> 
>>> Is deferring this conversation for the time being acceptable to everyone?
>> 
>> First, let’s review why it’s important to address this eventually:
>> 
>> (1) A number proposed improvements to the language have been opposed partly on the basis of round-trippability. But the claims for what needs to be round tripped have not been substantiated, and it’s not even entirely clear what properties need to be preserved. 
>> 
>> That makes it hard to have a meaningful conversation about these issues, or to come to a final resolution. We at Apple feel strongly that some tweaks need to be made for human-writability. However, if certain aspects of round trippability were shown to be performance-essential, we would accept that as a constraint on language improvement proposals.
>> 
>> Referring to non-specific feedback from developers, or saying it’s a “hard requirement” does not help advance this conversation.
>> 
>> (2) Myles is a co-editor of the WGSL specification. He needs to understand exactly what the constraints on the language are, and why they are that way, in order to perform that role effectively. We can’t expect him to just ask a Googler every time.
>> 
>> (3) While we’ve agreed to define WGSL in terms of SPIR-V semantics, we have not agreed that WebGPU performance on top of Vulkan is more important than performance on top of Metal or DirectX. We are concerned about the SPIR-V-centricity of the performance conversation. MSL and HLSL are equally first class targets in terms of performance. Without knowing the details, it’s hard to gauge whether we are missing something for these other target APIs and languages.
>> 
>> 
>> It’s reasonable to fill in details in the spec first, before getting to these issues. The current version is clearly incomplete. (Though note, anything that’s a substantial change should probably be reviewed by the CG or at least by the co-editors at this point).
>> 
>> But we can’t put off these conversations forever. With that in mind, could you propose a time frame for when we can come back to this topic?
>> 
>> 
>> Regards,
>> Maciej
>> 
>>> 
>>> 
>>> 
>>> regards,
>>> david
>>> 
>>> 
>>> 
>>> 
>>> On Sun, Mar 29, 2020 at 7:40 AM Myles C. Maxfield <mmaxfield@apple.com <mailto:mmaxfield@apple.com>> wrote:
>>> Hi!
>>> 
>>> There has been a fair amount of discussion <https://github.com/gpuweb/gpuweb/issues/622> recently about the process of round-tripping SPIR-V -> WGSL -> SPIR-V. In particular, this round-tripping operation may introduce <https://github.com/gpuweb/gpuweb/issues/579> some modifications to the original source which were not present <https://github.com/gpuweb/gpuweb/issues/581> in the original source. This group has already agreed that the round-trip will not be byte-for-byte identical <https://docs.google.com/document/d/15Pi1fYzr5F-aP92mosOLtRL4qLczmVRWQaomfQmGNzo/edit> to the original source. However, the WGSL group hasn’t come up with a set of criteria for whether not a transformation is too disruptive to the original source to introduce.
>>> 
>>> It is also accepted practice in WebGL for certain browsers to reject specific versions of certain drivers which are known to have bugs in them. In addition, the WebGPU group has already required certain common Vulkan extensions, thereby effectively rejecting specific Vulkan drivers which don’t support those common extensions.
>>> 
>>> With all that in mind, I’d like to propose the following test to determine whether a particular construct in the source SPIR-V program must be required to be unperturbed in the round-tripped SPIR-V program:
>>> A class of shaders exists, written in a human-writable shading language, which a human might have reasonably written
>>> … when compiled from that human-writable language into SPIR-V using the latest version of a popular compiler, and then the construct is perturbed (e.g. a variable hoisted, or a const promoted to a variable, or duplicate code moved to a common function, etc.),
>>> … it results in a reproducible and significant performance or correctness problem
>>> … on a shipping system configuration of the most recent version of a major browser (because only new browser versions will get WebGPU)
>>> … which has a significant number of users
>>> 
>>> If passing this test can be demonstrated, and the specific performance results or incorrect behavior can be displayed publicly (preferably on a WebGPU GitHub issue), then this is a compelling case to redesign WGSL such that it doesn’t require this specific perturbation.
>>> 
>>> This also goes for HLSL and MSL. If compiling to HLSL or MSL produces a problem which passes this test, then that is is a compelling case to redesign WGSL such that it doesn’t require this problem.
>>> 
>>> Thanks,
>>> Myles and the rest of the WebKit WebGPU team
>> 
>