Recent Truffle DSL changes.

Christian Humer christian.humer at gmail.com
Thu Sep 18 16:09:28 UTC 2014 

Hi Stefan,

So, I was wondering whether the brute-force approaches is intended, or
> whether there is an easy way around by for instance telling the DSL that
> doObject is the next general thing after doBool, doLong, or doDouble
> respectively.


Unfortunately there is no way to resolve this yet. But I am aware of that
problem. We could do this if we declare a hint where to rewrite to next
from all compatible specializations. Its on my TODO list.I already talked
to Andreas a few weeks ago and we also should not use rewrite by exception
for local variables. So we might invent a new pattern for that.

Does any of that make sense, or am I missing something?


Yes this needs to be done at specialization level. The DSL implementation
does this on this level (with my patch). All the inline cache
implementation need to do the same. This is why we currently expose the
"atomic(Callable)" API. Also nodes should not be respecialized twice as you
mentioned. Instead they should be copied.




- Christian Humer

On Thu, Sep 18, 2014 at 4:28 PM, Stefan Marr <java at stefan-marr.de> wrote:

> Hi Christian:
>
> Trying to figure out what the issue could be, I am wondering what the idea
> behind the replaceHelper logic and the difference between adoptHelper(.)
> and adoptUnadoptedHelper(.) is.
>
> Somehow, there seem to be situations in which the resulting tree contains
> nodes for which the parent link has not been set.
>
> I am also wondering, whether the approach of synchronizing on the level of
> the rewrite operations itself is reliable enough or whether the
> synchronization should happen one level up, i.e., the specialization logic
> might need to ensure that each node is only specialized exactly once.
> Otherwise, two threads could end up specializing the tree multiple times
> for the same node, racing for the replace. The thread that wins the race
> for the replace will continue executing a sub tree that is going to be
> removed from main tree once the second thread specialized the node. I
> assume, this could lead to all kind of strange and undesirable situations.
>
> Does any of that make sense, or am I missing something?
>
> Thanks
> Stefan
>
>
> On 18 Sep 2014, at 00:05, Christian Humer <christian.humer at gmail.com>
> wrote:
>
> > 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/
> >
> >
> >
> >
> > <dsl_thread_safety.diff>
>
> --
> Stefan Marr
> INRIA Lille - Nord Europe
> http://stefan-marr.de/research/
>
>
>
>

More information about the graal-dev mailing list