Skip to content

Make ptr load/store explicit with unary *. #1455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
kdashg wants to merge 1 commit into from
Closed

Make ptr load/store explicit with unary *. #1455

kdashg wants to merge 1 commit into from

Conversation

@kdashg
Copy link
Contributor

@kdashg kdashg commented Feb 21, 2021 •
edited
Loading

An alternative approach to #1334.

struct MyStruct {
  foo: i32;
};
fn example_store(v: ptr<private,i32>, s: ptr<private,MyStruct>) {
  *v = 12;
  *s.foo = 3;
}

fn bar(b: i32) {}
fn example_load(v: ptr<private,i32>, s: ptr<private,MyStruct>) {
  const a: i32 = *v;
  bar(*s.foo + 1);
}

fn bat(c: ptr<private,i32>) {}
fn example_copy(v: ptr<private,i32>) {
  const w: ptr<private,i32> = v;
  bat(v);
  bat(w); // Same effect as `bat(v)` again
}
fn get_or_init(a: ptr<in, i32>, c: i32) -> i32 {
  if (*a == 0) {
    *a = c;
  }
  return *a;
}

var<in> in_x : i32;
var<in> in_y : i32;
fn main() -> void {
  const x : ptr<in, i32> = in_x;
  const y : ptr<in, i32> = in_y;
  const effective_y = get_or_init(y, *x);
  [...]
}
for (var i : i32 = 0; i < *particleBuffer.particleCount; i = i + 1) {
 const particle = particleBuffer.particles[i]; // Subaccess(ptr<T>,K) -> ptr<typeof(T.K)>
 *particle.pos = *particle.pos + *particle.vel;
}

What's really cool about this is that all the load/store sites are obvious! "Wow I keep having to type star a lot" naturally pushes people to use intermediaries, which is what they should probably be doing anyway!

alan-baker
alan-baker reviewed Feb 22, 2021
View reviewed changes
wgsl/index.bs
<pre class='def'>
assignment_statement
: singular_expression EQUAL short_circuit_or_expression
: STAR singular_expression EQUAL short_circuit_or_expression
Copy link
Contributor

@alan-baker alan-baker Feb 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it intentional that all variables be treated as pointers? It would be worth adding that to the pointer operations examples and calling out in the variables section.

Copy link
Contributor Author

@kdashg kdashg Feb 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe accidental but that's the direction I'd prefer.

@dneto0
Copy link
Contributor

dneto0 commented Feb 22, 2021

Thanks for putting this together.

  • I like the explicitness. It's an advantage to see where memory operations occur.

  • Nit: The unary * operator should be low precedence:
    - *a.b should mean *(a.b)
    - Basically, any sub-access operation on a pointer takes precedence over dereferencing. So thats .field-access, .single-letter-vector-swizzle, and array-index.

  • Is the type-derivation for 'const' an integral part of this?
    - example const effective_y = get_or_init(y, *x); instead of const effective_y : ptr<in,i32> = get_or_init(y, *x)

  • It seems const-with-decl_storage is an alternate way of writing var. To me the var syntax is more clear about when the storage ends its lifetime: its lifetime ends when the declaration goes out of scope.

@dneto0
Copy link
Contributor

dneto0 commented Feb 22, 2021

Ah, I commented on the examples rather than the PR.
The PR makes it clear that the dereference is low precedence.

@kvark kvark mentioned this pull request Feb 22, 2021
Closed
@kvark
Copy link
Contributor

kvark commented Feb 22, 2021

I wrote down my thoughts on the matter in #1456. I'm not sure if this PR matches it.

@kainino0x kainino0x added the wgsl WebGPU Shading Language Issues label Mar 8, 2021
@kdashg
Copy link
Contributor Author

kdashg commented Apr 20, 2021

Closed by #1569.

@kdashg kdashg closed this Apr 20, 2021
@kainino0x kainino0x deleted the explicit-deref branch July 14, 2022 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alan-baker alan-baker alan-baker left review comments

Assignees

No one assigned

Labels

wgsl WebGPU Shading Language Issues

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@kdashg @dneto0 @kvark @alan-baker @kainino0x