Add support for atomic operations, excluding wait and notify, to singlepass. #831

nlewycky · 2019-09-24T22:46:44Z

Description

Adds support for atomic operations, excluding wait and notify, to singlepass. Enable with --enable-threads.

Review

Add a short description of the the change to the CHANGELOG.md file

nlewycky · 2019-09-24T22:47:33Z

bors try

bors · 2019-09-24T23:20:27Z

try

Build succeeded

wasmerio.wasmer

syrusakbary · 2019-09-24T23:58:45Z

lib/singlepass-backend/src/emitter_x64.rs

@@ -562,15 +566,15 @@ impl Emitter for Assembler {
            (Size::S64, Location::Memory(src, disp), Location::GPR(dst)) => {
                dynasm!(self ; lea Rq(dst as u8), [Rq(src as u8) + disp]);
            }
-            _ => unreachable!(),
+            _ => panic!("LEA {:?} {:?} {:?}", sz, src, dst),


Can you make the panics a bit more descriptive?

Just appending singlepass reached (or something similar)

Suggested change

_ => panic!("LEA {:?} {:?} {:?}", sz, src, dst),

_ => panic!("singlepass reached LEA {:?} {:?} {:?}", sz, src, dst),

losfair · 2019-09-29T04:56:50Z

lib/singlepass-backend/src/machine.rs

+    }
+
+    /// Specify that a given register is in use.
+    pub fn reserve_temp_gpr(&mut self, gpr: GPR) -> GPR {


Could you make the name a bit more distinguishable to acquire_temp_gpr, like reserve_unused_gpr ?

Done. I wanted to keep temp part of the name so that it refers to the RAX, RCX, RDX list of registers, not the RSI ... R11 list.

Okay, I thought this was used to reserve the RSI..R11 registers :)

Might be worth adding a comment explaining this if the new name doesn't make this explicit. As someone without context on this, I'm sure it'd take be a bit to figure this out

The new name is reserve_unused_temp_gpr. It also has a comment on the function, and an assertion in the function. I'm in the position where I know what it does, so I can't see what about it might be confusing. Let me know if I can improve it?

nlewycky · 2019-09-30T20:34:02Z

bors try

bors · 2019-09-30T21:16:55Z

try

Build succeeded

wasmerio.wasmer

nlewycky · 2019-10-01T19:27:11Z

Can I get an approval on this? Are @syrusakbary or @MarkMcCaskey OK with doing it or are we waiting for @losfair ?

MarkMcCaskey

looks good to me 👍

It's a shame we can't generate more of this code -- looks like there's a ton of patterns in the codegen part

MarkMcCaskey · 2019-10-01T20:32:20Z

lib/singlepass-backend/src/codegen_x64.rs

+                Location::GPR(tmp_aligncheck),
+            );
+            //a.emit_mov(Size::S64, Location::Imm64(align - 1), Location::GPR(tmp_mask));
+            //a.emit_and(Size::S64, Location::GPR(tmp_mask), Location::GPR(tmp_aligncheck));


Is this debug code? Probably best to remove it or explain in a comment why it's there

Deleted.

It's the old code. We're under heavy register pressure here, and I rewrote it to use one less register, but kept the original around in case my changes didn't work properly. Thanks!

nlewycky · 2019-10-01T21:09:07Z

bors r+

831: Add support for atomic operations, excluding wait and notify, to singlepass. r=nlewycky a=nlewycky # Description Adds support for atomic operations, excluding wait and notify, to singlepass. Enable with `--enable-threads`. # Review - [x] Add a short description of the the change to the CHANGELOG.md file Co-authored-by: Nick Lewycky <nick@wasmer.io>

bors · 2019-10-01T21:49:30Z

Build failed

wasmerio.wasmer

BUG: This might allocate RAX twice.

Adds the ability to reserve a specific temp-gpr register. Needed for CMPXCHG which always uses RAX.

Sizes are now ordered, to facilitate an assertion that one size is less (smaller) than another. panic! error messages are provided for remaining emitter functions.

nlewycky · 2019-10-02T23:39:08Z

bors r+

831: Add support for atomic operations, excluding wait and notify, to singlepass. r=nlewycky a=nlewycky # Description Adds support for atomic operations, excluding wait and notify, to singlepass. Enable with `--enable-threads`. # Review - [x] Add a short description of the the change to the CHANGELOG.md file Co-authored-by: Nick Lewycky <nick@wasmer.io>

bors · 2019-10-02T23:47:02Z

Canceled

nlewycky · 2019-10-02T23:47:19Z

bors r+

831: Add support for atomic operations, excluding wait and notify, to singlepass. r=nlewycky a=nlewycky # Description Adds support for atomic operations, excluding wait and notify, to singlepass. Enable with `--enable-threads`. # Review - [x] Add a short description of the the change to the CHANGELOG.md file Co-authored-by: Nick Lewycky <nick@wasmer.io> Co-authored-by: nlewycky <nick@wasmer.io>

bors · 2019-10-03T00:27:53Z

Build failed

wasmerio.wasmer

nlewycky · 2019-10-03T00:36:41Z

bors r+

831: Add support for atomic operations, excluding wait and notify, to singlepass. r=nlewycky a=nlewycky # Description Adds support for atomic operations, excluding wait and notify, to singlepass. Enable with `--enable-threads`. # Review - [x] Add a short description of the the change to the CHANGELOG.md file Co-authored-by: Nick Lewycky <nick@wasmer.io> Co-authored-by: nlewycky <nick@wasmer.io>

bors · 2019-10-03T01:14:20Z

Build succeeded

wasmerio.wasmer

nlewycky requested review from losfair, MarkMcCaskey and syrusakbary as code owners September 24, 2019 22:46

bors bot added a commit that referenced this pull request Sep 24, 2019

Try #831:

d1f4523

syrusakbary reviewed Sep 24, 2019

View reviewed changes

losfair reviewed Sep 29, 2019

View reviewed changes

nlewycky force-pushed the feature/singlepass-atomicops branch from be47a51 to 58cd7b9 Compare September 30, 2019 19:43

bors bot added a commit that referenced this pull request Sep 30, 2019

Try #831:

88aef06

MarkMcCaskey approved these changes Oct 1, 2019

View reviewed changes

nlewycky added 13 commits October 2, 2019 16:31

Initial implementation of atomic load/store and i32 atomic rmw add.

98f35ef

Add i32 rmw add and sub.

efc89e8

Add emitter for LOCK CMPXCHG so that we can emit compare-and-swap loops.

8189583

Use a compare-and-swap loop for AND.

6937019

BUG: This might allocate RAX twice.

Initial working implementation of I32AtomicRmwAnd!

cd1d06f

Adds the ability to reserve a specific temp-gpr register. Needed for CMPXCHG which always uses RAX.

Refactor out a compare-and-swap loop function.

f021d59

Add atomic.rmw operations, excluding xchg and cmpxchg.

bc7e017

Sizes are now ordered, to facilitate an assertion that one size is less (smaller) than another. panic! error messages are provided for remaining emitter functions.

Finish atomic operations for singlepass, excluding wait and notify.

ba68cfc

Add changelog entry.

e401f8e

Make the panics a bit more descriptive.

11c5e0d

Give this function a better name.

83b678b

cargo fmt

8e63d54

Delete dead (commented out) code. NFC.

ab76c23

nlewycky force-pushed the feature/singlepass-atomicops branch from 5445cde to ab76c23 Compare October 2, 2019 23:39

Merge branch 'master' into feature/singlepass-atomicops

f63c706

bors bot merged commit f63c706 into master Oct 3, 2019

bors bot deleted the feature/singlepass-atomicops branch October 3, 2019 01:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for atomic operations, excluding wait and notify, to singlepass. #831

Add support for atomic operations, excluding wait and notify, to singlepass. #831

nlewycky commented Sep 24, 2019 •

edited

Loading

nlewycky commented Sep 24, 2019

bors bot commented Sep 24, 2019

syrusakbary Sep 24, 2019 •

edited

Loading

nlewycky Sep 25, 2019

losfair Sep 29, 2019

nlewycky Sep 29, 2019

losfair Sep 29, 2019

MarkMcCaskey Sep 30, 2019

nlewycky Sep 30, 2019

nlewycky commented Sep 30, 2019

bors bot commented Sep 30, 2019

nlewycky commented Oct 1, 2019

MarkMcCaskey left a comment

MarkMcCaskey Oct 1, 2019

nlewycky Oct 1, 2019

nlewycky commented Oct 1, 2019

bors bot commented Oct 1, 2019

nlewycky commented Oct 2, 2019

bors bot commented Oct 2, 2019

nlewycky commented Oct 2, 2019

bors bot commented Oct 3, 2019

nlewycky commented Oct 3, 2019

bors bot commented Oct 3, 2019

	_ => panic!("LEA {:?} {:?} {:?}", sz, src, dst),
	_ => panic!("singlepass reached LEA {:?} {:?} {:?}", sz, src, dst),

Add support for atomic operations, excluding wait and notify, to singlepass. #831

Add support for atomic operations, excluding wait and notify, to singlepass. #831

Conversation

nlewycky commented Sep 24, 2019 • edited Loading

Description

Review

nlewycky commented Sep 24, 2019

bors bot commented Sep 24, 2019

try

Build succeeded

syrusakbary Sep 24, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nlewycky commented Sep 30, 2019

bors bot commented Sep 30, 2019

try

Build succeeded

nlewycky commented Oct 1, 2019

MarkMcCaskey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nlewycky commented Oct 1, 2019

bors bot commented Oct 1, 2019

Build failed

nlewycky commented Oct 2, 2019

bors bot commented Oct 2, 2019

Canceled

nlewycky commented Oct 2, 2019

bors bot commented Oct 3, 2019

Build failed

nlewycky commented Oct 3, 2019

bors bot commented Oct 3, 2019

Build succeeded

nlewycky commented Sep 24, 2019 •

edited

Loading

syrusakbary Sep 24, 2019 •

edited

Loading