Recent Truffle DSL changes.
Christian Humer
christian.humer at gmail.com
Wed Sep 17 22:05:09 UTC 2014
Hi Stefan,
Regarding the thread-safety issues. Yes polymorphic rewrites are not yet
thread-safe. But they were also not thread-safe in the old version. So you
might have been just lucky.
However it is straight forward to make them thread-safe. You just have to
wrap every replace in DSLShare into a closure and call atomic on the node
that is currently in the AST.
I've attached a patch. Let me know if that works for you. So I can push it
tomorrow. (Had no time to verify it yet)
Also be careful with the rewriting of function inline caches so that
replaces are always atomic. Also important is that you are not moving one
used AST node from one place to another. Instead copy it, remove the old
and insert the copy.
- Christian Humer
On Wed, Sep 17, 2014 at 8:57 PM, Stefan Marr <java at stefan-marr.de> wrote:
> Hi Christian:
>
> Now I am running into issues.
> I belief your new rewriting logic in the generated code for polymorphic
> nodes is not completely thread safe.
>
> Specifically, I am running into issues that a node is supposed to be
> rewritten, but not actually adopted yet.
>
> In case you might have a hunch, let me know. Will try to have a look later
> this week.
>
>
> To reproduce, you’ll need the thread-support branch and start a benchmark
> with at least 2 threads:
>
> git clone --recursive -b thread-support
> https://github.com/SOM-st/TruffleSOM.git
> cd TruffleSOM
> ant
> ./som.sh -cp Smalltalk Examples/Benchmarks/BenchmarkHarness.som Dispatch
> 10 0 10 2
>
> Best regards
> Stefan
>
>
> On 16 Sep 2014, at 23:57, Stefan Marr <java at stefan-marr.de> wrote:
>
> > Hi Christian:
> >
> > On 11 Aug 2014, at 19:59, Christian Humer <christian.humer at gmail.com>
> wrote:
> >
> >> I just pushed some new features to the DSL for the upcoming Truffle
> >> release. Please note that some of the changes may break existing Truffle
> >> interpreters. Guest languages that don't use Truffle DSL are not
> affected
> >> by this push.
> >
> > I finally found the time to update Graal.
> > Looks good. Didn’t had any major issues. Performance seems to remain
> more or less the same.
> >
> > Only got two nits. The first one is that the generated code does have
> one simple issue with the code style, which can be solved by initializing
> the method body unconditionally (see patch below).
> > The second thing is a simple typo in one of the error messages.
> >
> > Best regards
> > Stefan
> >
> > diff --git
> a/graal/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/generator/NodeCodeGenerator.java
> b/graal/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/generator/NodeCodeGenerator.java
> > index 0a38640..c3bc5eb 100644
> > ---
> a/graal/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/generator/NodeCodeGenerator.java
> > +++
> b/graal/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/generator/NodeCodeGenerator.java
> > @@ -906,9 +906,9 @@ public class NodeCodeGenerator extends
> AbstractCompilationUnitFactory<NodeData>
> > CodeExecutableElement method = new
> CodeExecutableElement(modifiers(PUBLIC), context.getType(void.class),
> "updateTypes0");
> > method.getParameters().add(new
> CodeVariableElement(classArray, "types"));
> >
> > - if (getModel().isPolymorphic()) {
> > - CodeTreeBuilder builder = method.createBuilder();
> > + CodeTreeBuilder builder = method.createBuilder();
> >
> > + if (getModel().isPolymorphic()) {
> > int index = 0;
> > for (NodeExecutionData execution :
> getModel().getNode().getChildExecutions()) {
> > String fieldName = polymorphicTypeName(execution);
> > diff --git
> a/graal/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/parser/NodeParser.java
> b/graal/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/parser/NodeParser.java
> > index 202815e..516dc18 100644
> > ---
> a/graal/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/parser/NodeParser.java
> > +++
> b/graal/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/parser/NodeParser.java
> > @@ -650,7 +650,7 @@ public class NodeParser extends
> AbstractParser<NodeData> {
> > SpecializationData next = i + 1 < specializations.size() ?
> specializations.get(i + 1) : null;
> >
> > if (!cur.isContainedBy(next)) {
> > - next.addError("This specialiation is not a valid
> exceptional rewrite target for %s. To fix this make %s compatible to %s or
> remove the exceptional rewrite.",
> > + next.addError("This specialization is not a valid
> exceptional rewrite target for %s. To fix this make %s compatible to %s or
> remove the exceptional rewrite.",
> > cur.createReferenceName(), next != null
> ? next.createReferenceName() : "-", cur.createReferenceName());
> > continue;
> > }
> > --
> > 1.9.1
> >
> >
> >
> >
> > --
> > Stefan Marr
> > INRIA Lille - Nord Europe
> > http://stefan-marr.de/research/
> >
> >
> >
>
> --
> Stefan Marr
> INRIA Lille - Nord Europe
> http://stefan-marr.de/research/
>
>
>
>
-------------- next part --------------
# HG changeset patch
# Parent b391c2f1901665081ee685a8416c27ff87cd4ddd
Truffle-DSL: added thread-safety for DSL rewrites.
diff -r b391c2f19016 -r 6817f289f27a graal/com.oracle.truffle.api.dsl/src/com/oracle/truffle/api/dsl/internal/DSLShare.java
--- a/graal/com.oracle.truffle.api.dsl/src/com/oracle/truffle/api/dsl/internal/DSLShare.java Fri Sep 05 00:08:16 2014 +0200
+++ b/graal/com.oracle.truffle.api.dsl/src/com/oracle/truffle/api/dsl/internal/DSLShare.java Wed Sep 17 23:57:33 2014 +0200
@@ -25,6 +25,7 @@
package com.oracle.truffle.api.dsl.internal;
import java.util.*;
+import java.util.concurrent.*;
import com.oracle.truffle.api.nodes.*;
@@ -50,19 +51,23 @@
return containsClass(newNode.getMetadata0().getIncludes(), oldNode);
}
- public static <T extends Node & DSLNode> T rewrite(Node thisNode, T newNode, String message) {
- assert newNode != null;
- if (getNext(thisNode) != null || getPrevious(thisNode) != null) {
- // already polymorphic -> append
- return appendPolymorphic(findUninitialized(thisNode), newNode);
- } else if (includes(thisNode, newNode)) {
- // included -> remains monomorphic
- newNode.adoptChildren0(thisNode, null);
- return thisNode.replace(newNode, message);
- } else {
- // goto polymorphic
- return null;
- }
+ public static <T extends Node & DSLNode> T rewrite(final Node thisNode, final T newNode, final String message) {
+ return thisNode.atomic(new Callable<T>() {
+ public T call() {
+ assert newNode != null;
+ if (getNext(thisNode) != null || getPrevious(thisNode) != null) {
+ // already polymorphic -> append
+ return appendPolymorphic(findUninitialized(thisNode), newNode);
+ } else if (includes(thisNode, newNode)) {
+ // included -> remains monomorphic
+ newNode.adoptChildren0(thisNode, null);
+ return thisNode.replace(newNode, message);
+ } else {
+ // goto polymorphic
+ return null;
+ }
+ }
+ });
}
@SuppressWarnings("unchecked")
@@ -86,37 +91,47 @@
return cur;
}
- public static <T extends Node & DSLNode> T rewriteUninitialized(Node uninitialized, T newNode) {
- Node prev = getPrevious(uninitialized);
- if (prev == null) {
- newNode.adoptChildren0(uninitialized, null);
- return uninitialized.replace(newNode, "Uninitialized monomorphic");
- } else {
- return appendPolymorphic(uninitialized, newNode);
- }
+ public static <T extends Node & DSLNode> T rewriteUninitialized(final Node uninitialized, final T newNode) {
+ return uninitialized.atomic(new Callable<T>() {
+ public T call() {
+ Node prev = getPrevious(uninitialized);
+ if (prev == null) {
+ newNode.adoptChildren0(uninitialized, null);
+ return uninitialized.replace(newNode, "Uninitialized monomorphic");
+ } else {
+ return appendPolymorphic(uninitialized, newNode);
+ }
+ }
+ });
+
}
- public static <T extends Node & DSLNode> T rewriteToPolymorphic(Node oldNode, DSLNode uninitializedDSL, T polymorphic, DSLNode currentCopy, DSLNode newNodeDSL, String message) {
- assert getNext(oldNode) == null;
- assert getPrevious(oldNode) == null;
- assert newNodeDSL != null;
+ public static <T extends Node & DSLNode> T rewriteToPolymorphic(final Node oldNode, final DSLNode uninitializedDSL, final T polymorphic, final DSLNode currentCopy, final DSLNode newNodeDSL,
+ final String message) {
+ return oldNode.atomic(new Callable<T>() {
+ public T call() {
+ assert getNext(oldNode) == null;
+ assert getPrevious(oldNode) == null;
+ assert newNodeDSL != null;
- Node uninitialized = (Node) uninitializedDSL;
- Node newNode = (Node) newNodeDSL;
- polymorphic.adoptChildren0(oldNode, (Node) currentCopy);
+ Node uninitialized = (Node) uninitializedDSL;
+ Node newNode = (Node) newNodeDSL;
+ polymorphic.adoptChildren0(oldNode, (Node) currentCopy);
- updateSourceSection(oldNode, uninitialized);
- // new specialization
- updateSourceSection(oldNode, newNode);
- newNodeDSL.adoptChildren0(null, uninitialized);
- currentCopy.adoptChildren0(null, newNode);
+ updateSourceSection(oldNode, uninitialized);
+ // new specialization
+ updateSourceSection(oldNode, newNode);
+ newNodeDSL.adoptChildren0(null, uninitialized);
+ currentCopy.adoptChildren0(null, newNode);
- oldNode.replace(polymorphic, message);
+ oldNode.replace(polymorphic, message);
- assert polymorphic.getNext0() == currentCopy;
- assert newNode != null ? currentCopy.getNext0() == newNode : currentCopy.getNext0() == uninitialized;
- assert uninitializedDSL.getNext0() == null;
- return polymorphic;
+ assert polymorphic.getNext0() == currentCopy;
+ assert newNode != null ? currentCopy.getNext0() == newNode : currentCopy.getNext0() == uninitialized;
+ assert uninitializedDSL.getNext0() == null;
+ return polymorphic;
+ }
+ });
}
private static void updateSourceSection(Node oldNode, Node newNode) {
More information about the graal-dev
mailing list