I think that we do need operator[] and operator[]=, unlike what I concluded in https://bugs.webkit.org/show_bug.cgi?id=176958#c2 and subsequently landed. Otherwise, what does this return: ? operator&.xz(thread float4^) It cannot return float2, because it needs to return a pointer to a field. It cannot return float2^, because xz are not next to each other. But it would work if we could instead override: float2 operator.xz(float4) void operator.xz=(float4, float2)
Here are some notes I took while pondering this. I think I have an algorithm now. -- array[index] += foo(); 1. ptr = &array[index]; 2. load from ptr 3. foo() 4. store to ptr native struct Foo; native struct Bar; native struct Baz; Bar operator.bar(Foo); Foo operator.bar=(Foo, Bar); thread Baz^ operator&.baz(thread Foo^); Foo foo; RMW(foo.bar, anonVar<0>, anonVar<1>, newValueExp(anonVar<0>), anonVar<1>) => RMW(foo, anonVar<2>, anonVar<3>, (anonVar<0>, anonVar<1>, anonVar<0> = operator.bar(anonVar<2>), anonVar<1> = newValueExp(anonVar<0>), operator.bar=(anonVar<2>, anonVar<1>)), anonVar<1>) RMW(foo.baz, anonVar<0>, anonVar<1>, newValueExp(anonVar<0>), anonVar<1>) => (anonVar<0>, anonVar<1>, anonVar<2>, anonVar<2> = &foo.baz, anonVar<0> = ^anonVar<2>, anonVar<1> = newValueExp(anonVar<0>), ^anonVar<2> = anonVar<1>) native struct Thingy; Thingy operator.thingy(Bar); Bar operator.thingy=(Bar, Thingy); RMW(foo.bar.thingy, anonVar<0>, anonVar<1>, newValueExp(anonVar<0>), anonVar<1>) => RMW(foo.bar, anonVar<2>, anonVar<3>, (anonVar<0>, anonVar<1>, anonVar<0> = operator.thingy(anonVar<2>), anonVar<1> = newValueExp(anonVar<0>), operator.thingy=(anonVar<2>, anonVar<1>)), anonVar<1>) => RMW(foo, anonVar<4>, anonVar<5>, (anonVar<2>, anonVar<3>, anonVar<2> = operator.bar(anonVar<4>), anonVar<3> = (anonVar<0>, anonVar<1>, anonVar<0> = operator.thingy(anonVar<2>), anonVar<1> = newValueExp(anonVar<0>), operator.thingy=(anonVar<2>, anonVar<1>)), operator.bar=(anonVar<4>, anonVar<3>)), anonVar<1>) foo.bar = thingy => RMW(foo, anonVar<0>, anonVar<1>, (anonVar<2>, anonVar<2> = thingy, operator.bar=(anonVar<0>, anonVar<2>)), anonVar<2>) foo.baz = thingy => ^operator&.baz(&foo) = thingy foo.bar => operator.bar(foo) foo.baz => ^operator&.bar(&foo) foo.bar.thingy => operator.bar(foo).thingy => operator.thingy(operator.bar(foo)) foo.baz.stuff => ^operator&.baz(&foo) => ^operator&.stuff(operator&.baz(&foo)) load assign dereference rmw 1. figure out what the load conversion would do 2. if it yields a Dereference, then use the pointer deconstruction 3. otherwise use the normal deconstruction It's not clear if this deconstruction should happen in Checker or later. Might be easier if it happens later. That would require factoring visitCallExpression out. That shouldn't be hard. First the checker would do its thing pretending that DotExpression and IndexExpression are just valid lvalues. Then a subsequent pass will convert those into calls or other things depending on context. This would mean that the only places that have to know the & and @ tricks that CallExpression currently does are the places that handle the load case. We'll need both IndexExpression and DotExpression to know how to do it.
Created attachment 321007 [details] more
Created attachment 321010 [details] most of it is written This isn't done yet. I still have to drag RMWExpression and IndexExpression through all of the visitors. But, I think the hard parts are done.
Created attachment 321017 [details] starting to test it
Created attachment 321018 [details] it passes so many tests
Created attachment 321019 [details] passes all of our current tests
Created attachment 321023 [details] it's passing significant tests Passes all old tests plus some large new ones.
Created attachment 321025 [details] the patch
Created attachment 321050 [details] added another substantial test
Created attachment 321052 [details] more tests
Created attachment 321053 [details] the patch Even more crazy testing.
Created attachment 321057 [details] wrote more tests
Created attachment 321058 [details] more tests One easy way to create new tests is to take an existing test and try to make its functions as generic as possible.
Created attachment 321059 [details] the patch
Created attachment 321076 [details] the patch
Created attachment 321106 [details] the patch
Comment on attachment 321106 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=321106&action=review Lots of this is over my head for now, first look at WSL :) r=me with that caveat. > Tools/WebGPUShadingLanguageRI/Checker.js:40 > } Ah no semicolon! > Tools/WebGPUShadingLanguageRI/Checker.js:160 > + throw new WTypeError(node.origin.originString, "LHS of read-modify-write is not an LValue: " + node.lVale); Typo "node.lVale" > Tools/WebGPUShadingLanguageRI/Checker.js:236 > + throw new Error("Cannot get typeForAnd"); Is there some context that can be thrown here? Looked like you have origin laying around that helps figure out where in the source this comes from? > Tools/WebGPUShadingLanguageRI/FindHighZombies.js:27 > +function findHighZombies(program) 👍 > Tools/WebGPUShadingLanguageRI/Parse.js:784 > + let token = consume("+", "-", "*", "/", "%", "^", "&", "|", "<", ">", "<=", ">=", "==", "++", "--", "&", ".", "~", "<<", ">>", "["); Does order in "consume" matter here? Because you have "<" before "<<", would it consume "<" first? I guess you already had "-" before "--" so no? You also have "&" twice.
(In reply to JF Bastien from comment #17) > Comment on attachment 321106 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=321106&action=review > > Lots of this is over my head for now, first look at WSL :) > > r=me with that caveat. > > > Tools/WebGPUShadingLanguageRI/Checker.js:40 > > } > > Ah no semicolon! > > > Tools/WebGPUShadingLanguageRI/Checker.js:160 > > + throw new WTypeError(node.origin.originString, "LHS of read-modify-write is not an LValue: " + node.lVale); > > Typo "node.lVale" fixed > > > Tools/WebGPUShadingLanguageRI/Checker.js:236 > > + throw new Error("Cannot get typeForAnd"); > > Is there some context that can be thrown here? Looked like you have origin > laying around that helps figure out where in the source this comes from? We throw Error when it's an ICE, not an error to be reported to the user. I put this in there as basically an assertion; I don't expect it to ever fire. > > > Tools/WebGPUShadingLanguageRI/FindHighZombies.js:27 > > +function findHighZombies(program) > > 👍 > > > Tools/WebGPUShadingLanguageRI/Parse.js:784 > > + let token = consume("+", "-", "*", "/", "%", "^", "&", "|", "<", ">", "<=", ">=", "==", "++", "--", "&", ".", "~", "<<", ">>", "["); > > Does order in "consume" matter here? Because you have "<" before "<<", would > it consume "<" first? I guess you already had "-" before "--" so no? > > You also have "&" twice. The order doesn't matter. The lexer takes care of having the right regexp's to parse these. I'll clean up this list.
Landed in https://trac.webkit.org/changeset/222169/webkit
<rdar://problem/34693376>
Migrated to https://github.com/gpuweb/WHLSL/issues/131