Skip to content

Always register non-MVP types and check on actual use #1194

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.

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

Merged
merged 2 commits into from
Mar 28, 2020
Merged

Conversation

dcodeIO
Copy link
Member

@dcodeIO dcodeIO commented Mar 27, 2020

This PR should fix #1162 in that v128 and anyref will always be present, even if the respective feature is not enabled, so these can be used conditionally. Chances are that I am missing diagnostics somewhere, but general strategy is to check on use of a storage type

  • Declaring a local of that type
  • Declaring a global of that type
  • Reading or writing a field of that type
  • Compiling a function with a respective this, parameter or return type
  • Instantiating a class with a field of that type

and when a new value of the type is created by one of the respective builtins, which should also cover all sorts of inference if no type is annotated on a storage type. That should give us the minimum amount of diagnostics, instead of erroring on every single compiled expression resolving to that type, along minimal checking overhead.

Unless discovered in review, we'll notice that a diagnostic is missing if Binaryen fails to validate a module due to unexpected types.

@jtenner
Copy link
Contributor

jtenner commented Mar 28, 2020

Should I validate that this works locally?

@dcodeIO
Copy link
Member Author

dcodeIO commented Mar 28, 2020

Would be great :)

@jtenner
Copy link
Contributor

jtenner commented Mar 28, 2020

After waiting about ten minutes for the dist files to generate, it appears this works locally for me.

class Test {
  a: f32 = 0.0;
  b: f32 = 0.0;
  c: f32 = 0.0;
  d: f32 = 0.0;
  get abcd(): v128 {
    return v128.load(changetype<usize>(this), offsetof<Test>("a"));
  }
  set abcd(value: v128) {
    v128.store(changetype<usize>(this), value, offsetof<Test>("a"));
  }
}

let a = new Test();
if (ASC_FEATURE_SIMD) {
  let vec = v128.mul<f32>(a.abcd, v128.splat<f32>(2));
  a.abcd = vec;
} else {
  a.a *= 2;
  a.b *= 2;
  a.c *= 2;
  a.d *= 2;
}

It compiles when simd is enabled and when simd is not enabled.

Thanks for the fix.

@dcodeIO dcodeIO merged commit 08075c6 into master Mar 28, 2020
@dcodeIO dcodeIO deleted the issue-1162 branch May 14, 2020 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Properties that return v128 cause compiler error even though they are conditionally used
2 participants