Pthreads #67

juj · 2015-03-17T17:24:47Z

Adds LLVM side support for compiling for the pthreads/atomics support.

This set of commits implements initial pthreads library support for Emscripten code, and allows native code to utilize multithreading with the pthreads API. This backs on experimental research on the JavaScript SharedArrayBuffer and Atomics APIs from https://docs.google.com/document/d/1NDGA_gZJ7M7w1Bh8S0AoDyEqwDdRh4uSoTPSNn77PFk/edit?usp=sharing , and as such is preliminary and unsupported in other browsers except current Firefox Nightly builds. By default pthreads support is disabled, and the feature must be explicitly enabled by specifying the compiler flag -s USE_PTHREADS=1 and the linker flag -lpthread.

kripken · 2015-03-23T20:35:12Z

lib/Target/JSBackend/JSBackend.cpp

@@ -855,15 +927,36 @@ std::string JSWriter::getLoad(const Instruction *I, const Value *P, Type *T, uns
  return text;
 }

+static const std::string AtomicsStore = "Atomics_store(";


Is this a useful optimization here? Is there a specific reason why it is done on this string and not others?

Hmm, let me just remove this, it is a leftover from previous form of code. I hope compilers are smart to optimize this on their own anyways.

juj · 2015-03-24T16:21:43Z

Addressed the review comments at juj/emscripten-fastcomp@a5b54c1...0bc3f1a .

kripken · 2015-05-29T18:48:05Z

lib/Target/JSBackend/CallHandlers.h

+std::string getAddressAsString(const Value *Ptr, int shift) {
+  Type *t = cast<PointerType>(Ptr->getType())->getElementType();
+  if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(Ptr)) {
+    return utostr(getGlobalAddress(GV->getName().str()) >> shift);


getGlobalAddress returns a number. that number does not include a relocation offset, which we need in shared modules. relocateGlobal will do a relocation for you (it operates on a string, so need to take into account the shift on the extra part here).

Ok, fixed. Although note that threads and shared libraries is not supported at present, that is left for later.

…ead.

…guarantee that SpiderMonkey will not optimize those instructions out when executing, and to offer x86-like strong memory consistency across code that uses such constructs.

…ontrol whether targeting multithreading with Atomics and SAB or not.

…support enabled.

…h pthreads enabled.

…hg support to UsedVars in JSBackend.cpp.

…ith pthreads enabled in JSBackend.cpp.

…4 types.

… GCC intrinsics to Emscripten library function calls. Mark 64-bit AtomicCmpXchg instructions unsupported at the moment.

…und, do it as u32 version with a CAS loop. TODO: revert this commit once https://bugzilla.mozilla.org/show_bug.cgi?id=1141986 lands.

…nd unsupported f32 and f64 atomic cas ops.

…emory addresses.

juj · 2015-06-01T12:16:10Z

Addressed the review comments and rebased on top of the latest. This should hopefully look simpler now?

kripken · 2015-06-01T17:18:44Z

lib/Target/JSBackend/CallHandlers.h

@@ -549,6 +554,154 @@ DEF_CALL_HANDLER(emscripten_asm_const_double, {
  return getAssign(CI) + getCast(handleAsmConst(CI), Type::getDoubleTy(CI->getContext()));
 })

+std::string getAddressAsString(const Value *Ptr, int shift) {


Let's call this getShiftedPtr, as it works on pointers, and it doesn't return just a string, but specifically it returns a shifted result (so it isn't the real pointer/address anymore).

Also, please move this to right before getPtrUse in the main JSBackend.cpp file. Then the distinction between the two methods is clear.

And once at that location, it looks like we can remove getHeapAccess, and instead implement getPtrUse using the new getShiftedPtr (the only extra thing needed is HEAP?[ on the left and ] on the right, can do that in getPtrUse, or alternatively leave getHeapAccess but without the shifting inside it, so all it does is add the HEAP?[ and ]; I am ok with either).

Oh, I see there is getHeapName, so can use that for the HEAP? part, leaving just [ and ].

Or actually instead of everything I said, this method could be implemented using the getHeapNameAndIndex family of methods.

Anyhow, sorry to ramble here, but in summary

I think getShiftedPtr is a better name

I'd prefer this to be implemented using other methods, without a special check on GlobalVariable, and without the need to relocateGlobal, as the other methods already do both.

Ok, updated the pull request to rename this to getShiftedPtr and to reuse the existing code as much as possible.

…tation from the existing functions.

kripken · 2015-06-01T20:34:22Z

Looks nice, thanks.

One last thing, just to be sure, please test compilation time before and after this pull. I'm not too worried, but still this changes a lot of the string handling code in emitting pointer uses in the backend. A quick test on a large codebase of js backend time, with and without this pull, should be enough to check.

If that is ok, then lgtm.

juj · 2015-06-02T17:59:25Z

I rebuilt Unity3D three times with trunk and both pthreads PRs applied, with the following results:

incoming:
emscript: llvm backend took 11.1803879738 seconds
emscript: llvm backend took 10.9915230274 seconds
emscript: llvm backend took 10.9425158501 seconds

pthreads:
emscript: llvm backend took 11.1047959328 seconds
emscript: llvm backend took 11.1267051697 seconds
emscript: llvm backend took 11.0028831959 seconds

so the differences do look quite negligible. I'll go ahead and merge these in, if everything is set?

kripken · 2015-06-03T00:07:21Z

Sure!

Pthreads

juj mentioned this pull request Mar 17, 2015

Pthreads emscripten-core/emscripten#3266

Merged

kripken reviewed Mar 23, 2015
View reviewed changes

juj force-pushed the pthreads branch from 0bc3f1a to 07ea482 Compare March 25, 2015 10:32

juj force-pushed the pthreads branch from 07ea482 to b9e66ed Compare April 13, 2015 14:43

juj force-pushed the pthreads branch 4 times, most recently from ba7d9a4 to a2bf439 Compare May 5, 2015 19:08

juj force-pushed the pthreads branch from 8a43284 to c565f2d Compare May 15, 2015 12:21

juj force-pushed the pthreads branch from c565f2d to 35794dc Compare May 27, 2015 12:42

kripken reviewed May 29, 2015
View reviewed changes

juj added 6 commits June 1, 2015 15:07

Make memory initializer allocation aware of whether running in a pthr…

3294faa

…ead.

Implement volatile loads and stores to use Atomics.load and store to …

2c387ab

…guarantee that SpiderMonkey will not optimize those instructions out when executing, and to offer x86-like strong memory consistency across code that uses such constructs.

Add support for LLVM atomics.

ecf31f6

Implement Atomic compare-and-exchange support.

9198866

Implement Atomic fence support

b4a3db7

Add support for AtomicCmpXchg instruction for pthreads.

1ed5faa

juj added 20 commits June 1, 2015 15:07

Add a backend command line parameter -emscripten-enable-pthreads to c…

a0804f0

…ontrol whether targeting multithreading with Atomics and SAB or not.

Implement support for AtomicCmpXchg when not compiling with pthreads …

b4bcbe2

…support enabled.

Add missing check for not emitting atomic loads when not building wit…

9e2c3ab

…h pthreads enabled.

Apply review formatting.

61e6276

Register the variables required to implement Instruction::AtomicCmpXc…

402699e

…hg support to UsedVars in JSBackend.cpp.

Align whitespace in AtomicRMWInst switch-cases in JSBackend.cpp.

0b4e68c

Clarify comment in Instruction::Fence() for JSBackend.cpp.

173e6f6

Do not emit "if (!ENVIRONMENT_IS_PTHREAD)" at all when not building w…

900826d

…ith pthreads enabled in JSBackend.cpp.

Fix asm.js validation when code emits atomic operations to f32 and f6…

dac3535

…4 types.

Add support for more emulated 64bit operations.

afb0efb

Add support for expanding 64-bit atomic add and sub instructions from…

7ec89a9

… GCC intrinsics to Emscripten library function calls. Mark 64-bit AtomicCmpXchg instructions unsupported at the moment.

Fix asm.js validation of 64-bit atomic stores.

c4ad168

Add support for asm.js validating emscripten_atomic_exchange intrinsics.

91a6044

Fix emscripten atomic exchange to call correct atomic intrinsic.

25ecc3a

Fix the number of params to emscripten_atomic_exchange intrinsic.

ff25c5e

HACK: Atomics.exchange() is not yet implemented, so as a temp workaro…

91f7005

…und, do it as u32 version with a CAS loop. TODO: revert this commit once https://bugzilla.mozilla.org/show_bug.cgi?id=1141986 lands.

Add missing relocateGlobal() in getAddressAsString(). Remove unused a…

ecc1252

…nd unsupported f32 and f64 atomic cas ops.

Remove code duplication between the different methods for accessing m…

3c942ae

…emory addresses.

Remove getByteAddressAsStr() as a duplicate of getValueAsStr().

91e09dc

Use PNaCl intrinsics based approach for handling atomic cmpxchg.

c439cf2

juj force-pushed the pthreads branch from cd521f2 to c439cf2 Compare June 1, 2015 12:08

kripken reviewed Jun 1, 2015
View reviewed changes

Rename getAddressAsString() to getShiftedPtr() and reuse its implemen…

63bf6b6

…tation from the existing functions.

juj added a commit that referenced this pull request Jun 3, 2015

Merge pull request #67 from juj/pthreads

4d008e1

Pthreads

juj merged commit 4d008e1 into emscripten-core:incoming Jun 3, 2015

RichardGale mentioned this pull request Aug 15, 2015

Examples on asm.js bkaradzic/bgfx#383

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pthreads #67

Pthreads #67

juj commented Mar 17, 2015

kripken Mar 23, 2015

juj Mar 24, 2015

juj commented Mar 24, 2015

kripken May 29, 2015

juj Jun 1, 2015

juj commented Jun 1, 2015

kripken Jun 1, 2015

kripken Jun 1, 2015

kripken Jun 1, 2015

kripken Jun 1, 2015

juj Jun 1, 2015

kripken commented Jun 1, 2015

juj commented Jun 2, 2015

kripken commented Jun 3, 2015

Pthreads #67

Pthreads #67

Conversation

juj commented Mar 17, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juj commented Mar 24, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juj commented Jun 1, 2015

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

kripken commented Jun 1, 2015

juj commented Jun 2, 2015

kripken commented Jun 3, 2015