Skip to content
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

Built-in atomic functions added #440

Merged
merged 7 commits into from
Feb 7, 2019
Merged

Built-in atomic functions added #440

merged 7 commits into from
Feb 7, 2019

Conversation

nidin
Copy link
Contributor

@nidin nidin commented Jan 28, 2019

Built-in atomic functions added for SharedArrayBuffer.

List of functions

Atomic.load<T>(offset: usize, constantOffset?: usize): T
Atomic.store<T>(offset: usize, value: void, constantOffset?: usize): void
Atomic.add<T>(ptr: usize, value: T, constantOffset?: usize): T
Atomic.sub<T>(ptr: usize, value: T, constantOffset?: usize): T
Atomic.and<T>(ptr: usize, value: T, constantOffset?: usize): T
Atomic.or<T>(ptr: usize, value: T, constantOffset?: usize): T
Atomic.xor<T>(ptr: usize, value: T, constantOffset?: usize): T
Atomic.xchg<T>(ptr: usize, value: T, constantOffset?: usize): T
Atomic.cmpxchg<T>(ptr: usize, expected:T, replacement: T, constantOffset?: usize): T
Atomic.wait<T>(ptr: usize, expected:T, timeout:i64): i32
Atomic.notify<T>(ptr: usize, count: u32): u32

nidin and others added 6 commits January 24, 2019 13:02
Extended `_BinaryenAddMemoryImport` and `_BinaryenSetMemory` functions with `shared: bool` argument
This will configure WASM memory as shared.
Updated AssemblyScript contributions author list
`shared:bool` argument added to `addMemoryImport` and  `setMemory` functions
@nidin nidin changed the title [Not ready] Shared memory part 2 Built-in atomic functions added Jan 28, 2019
@@ -2844,7 +3333,18 @@ function deferASM(
valueType: Type,
reportNode: Node
): ExpressionRef {
var prototype = assert(compiler.program.elementsLookup.get(name));
Copy link
Member

Choose a reason for hiding this comment

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

elementsLookup should also contain namespace members here, like both Atomic and Atomic.load etc. Is there a bug?

Copy link
Contributor Author

@nidin nidin Jan 28, 2019

Choose a reason for hiding this comment

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

I will cross check. Atomic.load was not in elementsLookup but it was a few months ago.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason namespace members are undefined in elementsLookup

compiler.program.elementsLookup.get("Atomic.load")

return undefined


compiler.program.elementsLookup.get("Atomic")

return

MapIterator {
  'load',
  'store',
  'add',
  'sub',
  'and',
  'or',
  'xor',
  'xchg',
  'cmpxchg',
  'wait',
  'notify' }

src/builtins.ts Outdated Show resolved Hide resolved
if (offset < 0) { // reported in evaluateConstantOffset
return module.createUnreachable();
}
compiler.currentType = typeArguments[0];
Copy link
Member

@MaxGraey MaxGraey Feb 1, 2019

Choose a reason for hiding this comment

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

It seems typeArguments[0] used 5 times so it will be great cache it to variable and may be in upper scope

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will change it.

if (!(typeArguments && typeArguments.length == 1)) {
compiler.error(
DiagnosticCode.Expected_0_type_arguments_but_got_1,
reportNode.range, "1", typeArguments ? typeArguments.length.toString(10) : "0"
Copy link
Member

Choose a reason for hiding this comment

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

.toString(10) is unnecessary. Just use toString()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This too.

@MaxGraey
Copy link
Member

MaxGraey commented Feb 6, 2019

btw you could try turn on threads for latest node via node --experimental-wasm-threads and write some tests.

@willemneal
Copy link
Contributor

How do threads work with node? In the Browser there are web workers, but I thought node was single threaded.

@MaxGraey
Copy link
Member

MaxGraey commented Feb 6, 2019

Mostly the same for now: https://nodejs.org/api/worker_threads.html

@nidin
Copy link
Contributor Author

nidin commented Feb 6, 2019

@MaxGraey This PR only includes built-in functions to make review easy. PR #278 Contains everything but it's too much for a single review. I am actually breaking the PR #278 into small parts.

@MaxGraey
Copy link
Member

MaxGraey commented Feb 6, 2019

I see. Totally makes sense. But also will great have small test cases of functionality present in this PR. Or add it in separate PR

@nidin
Copy link
Contributor Author

nidin commented Feb 6, 2019

Atomic operations won't compile without shared memory support in compiler. It will come in next PR. followed by test cases.

@MaxGraey
Copy link
Member

MaxGraey commented Feb 6, 2019

Got it!

@dcodeIO dcodeIO merged commit 7ce3296 into AssemblyScript:master Feb 7, 2019
@dcodeIO
Copy link
Member

dcodeIO commented Feb 7, 2019

Great, thanks! :)

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.

None yet

4 participants