Recent Truffle DSL changes.

[Wei Zhang]

Hi Christian,

Thanks for your explanation.

If I go back to using @ImplicitCast then we reopen the old discussion
we had a while ago...
Thing is @ImplicitCast makes specialization ambiguous as you suggested.

Say an AddNode taking a boolean and an int is specialized to doInt(int, int).
If the boolean became an int, the AddNode rewrite itself by looking
for a match from the *next* specialization available.
Consequently it will rewrite to doBigInteger, which is kind of wrong...

But for my case I guess I could only enable @ImplicitCast from boolean
to int. That should do it.
Just want to remind you a perhaps better way to implement type
coercion using Truffle DSL is very desirable ;).

Thanks,

On Thu, Oct 2, 2014 at 6:55 AM, Christian Humer
<christian.humer at gmail.com> wrote:
> Hi Wei,
>
>
> Yes. The rewriteOn feature got bit more restrictive with the new version.
> The requirement os that the next specialization must be strictly more
> generic than the specialization that uses rewritesOn. This requirement also
> takes into account implicit casts. For example if there is an implicit cast
> boolean to int. You can do the following:
>
>
> @Specialization(rewriteOn=Exception.class) int doBoolean(boolean operand)
> {...}
> @Specialization int doInt(int operand) {...}
>
>
>
> But if there is no implicit cast (like it is the case for your AddNode) you
> need to do the following:
>
> @Specialization(rewriteOn=Exception.class) int doBoolean(boolean operand)
> {...}
> @Specialization int doBooleanNoOverflow(boolean operand) {...}
> @Specialization int doInt(int operand) {...}
>
>
> I'd recommend you to add an implicit cast boolean -> int and define the
> specializations as follows:
>
>         @Specialization
>         int doBoolean(boolean left, boolean right) {
>             final int leftInt = left ? 1 : 0;
>             final int rightInt = right ? 1 : 0;
>             return leftInt + rightInt;
>         }
>
>         @Specialization(rewriteOn = ArithmeticException.class)
>         int doInteger(int left, int right) {
>             return ExactMath.addExact(left, right);
>         }
>
> I'd like to add that the new DSL version guarantees that the doBoolean
> specialization is always used if you get two boolean values as input.
> Previously the doInteger might also have been used in polymorphic cases.
> This is now fixed.
>
> Without implicit casts this definition gets quite cumbersome (previous
> definition also had its problems but the old DSL did not complain). To
> resolve it you need to define a follow up specialization with the same
> signature but without rewriteOn like I already described with my second
> example.
>
> After enabling all the implicit casts that were already defined in
> PythonTypes, I had these specializations left for the numeric types:
>
>         @Specialization
>         int doBoolean(boolean left, boolean right) {
>             final int leftInt = left ? 1 : 0;
>             final int rightInt = right ? 1 : 0;
>             return leftInt + rightInt;
>         }
>
>         @Specialization(rewriteOn = ArithmeticException.class)
>         int doInteger(int left, int right) {
>             return ExactMath.addExact(left, right);
>         }
>
>         @Specialization
>         BigInteger doBigInteger(BigInteger left, BigInteger right) {
>             return left.add(right);
>         }
>
>         @Specialization
>         double doDouble(double left, double right) {
>             return left + right;
>         }
>
>         @Specialization
>         PComplex doComplex(PComplex left, PComplex right) {
>             return left.add(right);
>         }
>
> I think this is as good as it gets. All the combination of types are now
> implicetly there.
>
> Let me know if this helps.
>
>
>
> - Christian Humer
>
> On Thu, Oct 2, 2014 at 12:58 AM, Wei Zhang <thezhangwei at gmail.com> wrote:
>
>> Hi Christian,
>>
>> I finally merged with graal-0.5... It was very smooth.
>> But I did run into one issue.
>>
>>
>> https://bitbucket.org/ssllab/zippy/src/cd9ba144cc20c30181f5314584e148ea38e1fa45/graal/edu.uci.python.nodes/src/edu/uci/python/nodes/expression/BinaryArithmeticNode.java?at=default
>> After merge in the AddNode of ZipPy, if I have multiple consecutive
>> specializations with the rewriteOn attribute specified to
>> ArithmeticException.class, the DSL processor complains.
>> I currently disabled rewriteOn exception for doIntBoolean() and
>> doBooleanInt in AddNode to get around the error.
>> If I put them back on I get the following error msg:
>> This specialiation is not a valid exceptional rewrite target for
>> doBooleanInt(boolean, int). To fix this make doInteger(int, int)
>> compatible to doBooleanInt(boolean, int) or remove the exceptional
>> rewrite.
>>
>> Any though?
>>
>> Thanks,
>>
>> On Thu, Sep 18, 2014 at 10:00 AM, Stefan Marr <java at stefan-marr.de> wrote:
>> > Hi Christian:
>> >
>> > On 18 Sep 2014, at 18:09, Christian Humer <christian.humer at gmail.com>
>> wrote:
>> >
>> >> 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.
>> >
>> > Ok, I’ll keep it on my todo list as well.
>> >
>> >
>> >> 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.
>> >
>> > Will probably not get to making TruffleSOM thread-safe this week.
>> > But, I was wondering how the API could help me to know that I am doing
>> something in a way I shouldn’t do it.
>> >
>> > How about adding a ReentrantLock object to the root node, removing the
>> implicit lock acquisition from replace(), and instead putting an assertion
>> on isHeldByCurrentThread() in there? I realize that it might be slightly
>> inconvenient. But, it would make it explicit to the language implementer
>> that one has to look out for those things.
>> > As a compromise, a replaceLocked(.) could be introduced.
>> > Generally, I would also prefer if the method names would tell me
>> something about the concurrency properties.
>> >
>> > Another thing I was wondering about is whether some of those concepts in
>> DSLShare could be promoted for general use. It kind of represents good
>> standard practices, right?
>> > And even if one needs to step outside the DSL, I presume the general
>> structuring and concepts would still help to implement custom things. I was
>> thinking about the notion of ‘uninitialized’ and the find and rewrite
>> helper methods, for instance.
>> >
>> > Best regards
>> > Stefan
>> >
>> > --
>> > Stefan Marr
>> > INRIA Lille - Nord Europe
>> > http://stefan-marr.de/research/
>> >
>> >
>> >
>>