-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for handling primitives/builtin functions, and language settings #1
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Stefan for taking the initiative! I agree with the general idea, this is mainly the way primitives are being handled by SOMns, Mate, and now also SOM. I also think that primitives are a very basic block to work on. I added a couple of comments in the code just to contribute to the discussion.
* instance side. | ||
*/ | ||
boolean classSide() default false; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I am using the class name in Mate to simplify this. Because metaclasses are also standard objects. Is there any SOM-like language in which we would like to have classes as "second class citizens"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is mostly a design decision coming from the traditional SOM implementation.
I decided to keep that design instead of using the name as you did.
It doesn't make classes second-class citizens at all. It just combines loading of primitives for classes and their metaclasses.
And, with the SOM syntax for having both in the same lexical body, only separated by ----
, I think it's somewhat natural.
So, this is mostly for consistency with other SOMs.
src/bd/primitives/Primitive.java
Outdated
/** | ||
* Annotation to be applied on node classes that are meant to be used as primitive operations. | ||
* Thus, the node implements a built-in function of the interpreter, and might need to be | ||
* represented as a language-level method/function. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure what it is intended to mean since "Thus...".
The built-in function part is for the execute() methods? What does it mean that the node might need to be represented as a language-level method/function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to communicate what a primitive is.
The node for an +
might not only be an operator-like construct, but also a language level method. Since you can do 1 perform: #+ with: 3
, the +
also needs to be a language-level method, not just something operator-like.
Does that make sense?
Anything that could be done to clarify the comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, but actually the node having the annotation is not a language-level method. It is usually a kind of expression node. I still see the comment confusing and I a still do not have a better way to communicate the idea, sorry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the node is turned into a language-level method. There is a #+
that is built using the node. I would expect that to be the same thing in TruffleMate. That's how it works in SOMns and TruffleSOM anyway. See https://github.com/SOM-st/TruffleSOM/blob/bd-primitivies/src/som/primitives/Primitives.java#L197 There an SInvokable
is constructed from all nodes annotated with @Primitive
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know. What I mean is that the node in which the annotation is written is a ExpressionNode and the framework itself is in charge of creating an Invokable that includes that node. My point, and yes, it is just an opinion, is that someone not very involved with the framework, at a first read, may got confused and think that he should put the annotation in an Invokable like node.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I simplified the comment
@@ -0,0 +1,13 @@ | |||
package bd.settings; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure this file is part of this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I am not yet sure what to do about this.
This isn't ideal, but, the specializer somewhat depend on the setting for the DynamicMetric tool.
There is a todo, to think about better solutions. One option would be to have an interface for the context, and have the setting there. Either way, it needs additional infrastructure, which somewhat is accidental complexity for this PR, but needed to work in the context of SOMns.
if (prim.disabled() && VmSettings.DYNAMIC_METRICS) { | ||
return false; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ok, I see why we need VMSettings now. But, I do not think is worth having at this moment...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need it for SOMns...
ctorArgs[offset] = eagerWrapper ? null : argNodes[i]; | ||
offset += 1; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just an idea/suggestion, I think that it would be much more clear if we can separate eager primitive creation from standard specialization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you mean by that. Can you elaborate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eager primitives and standard method specialization at run time deal with arguments in different ways. For eagerWrappers we are sending null as argument. That may be confusing. Perhaps it would be clearer to separate the argument handling of eager primitives, as I said it is just an idea/suggestion that came into my mind while reading the code, nothing crucial.
src/bd/primitives/Specializer.java
Outdated
* @param <ExprT> the root type of expressions used by the language | ||
*/ | ||
public class Specializer<T, Context, ExprT> { | ||
protected final Context context; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the Truffle name for Context in this case is misleading. Perhaps something more in the sense of VM...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
VM is also such an overloaded term that it doesn't have any meaning anymore.
Yes, context is also overloaded, and it is also changing in the latest Truffle. Not really ideal. But, the objects I put in there are actually also Truffle context objects. So, it seems to be somewhat appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the problem here is that context has a very concrete meaning in ST and I see SOM as an ST-like language. So I'd avoid the Truffle dialect for this term. Anyway, it is just my opinion and actually I am using context also in Mate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Smalltalk term context
is non-standard compared to the rest of computer science. I'd rather go with frame
instead of Smalltalk's context
for Smalltalk, too.
And for BD, I'd rather stick with Truffle terminology. I'd think it makes it easier to match up concepts, since Truffle is a definite requirement anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Grace's spec uses neither of these terms, so I don't have any strong opinion. I suppose I'd prefer frame
out of the two (since it feels more general to me).
final NodeFactory<? extends ExprT> factory) { | ||
Class<?> nodeClass = factory.getNodeClass(); | ||
return nodeClass.getAnnotationsByType(Primitive.class); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure is the last two methods should be part of this class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not? It is common code used by the subclasses?
Where else would you put it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I do not have PrimiteLoader in Mate and here I am just browsing code so I do not know anything about the class hierarchy. But this two methods do not use anything about the PrimitiveLoader instance state, they seem quite independent. For the first one it may be part of the primitive? The second one perhaps here but static?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried putting getPrimitiveAnnotation
into @Primitive
but Java doesn't seem to allow static methods there.
Grace uses a prototype-based object model. Would it be possible to decouple support for primitives from class instances, so that the VM can support primitive expressions on any object? If it is possible to decouple support for primitives from class instances, then how significant are the changes that would be required to do so? It seems that If not possible, or if demands too much change, I'm happy to implement of a special class to distributes primitives (and make all grace objects a subclass of that special class). |
I worked a bit further on this, and made SOMns work with it. The idea is that the language can implement the |
295622a
to
98b3076
Compare
Ok, so, this is the first 'diamond' ready to merge, I think. @charig if I would want to apply it to TruffleMate, which branch would I be looking at? SOMns PR to use it is here: smarr/SOMns#185 |
This adds basic infrastructure to use annotations to handle primitives in Truffle languages. It supports parse-time and eager run-time specialization. During specialization, the normal method call can be replaced with a specific operation. This means, there is no overhead for a method call anymore in the interpreter. Signed-off-by: Stefan Marr <git@stefan-marr.de>
Signed-off-by: Stefan Marr <git@stefan-marr.de>
These config options aren’t necessary anymore. We are going to deal with these two options via an initializer method. Signed-off-by: Stefan Marr <git@stefan-marr.de>
Signed-off-by: Stefan Marr <git@stefan-marr.de>
Signed-off-by: Stefan Marr <git@stefan-marr.de>
Nodes might need access to the langauge’s state or behavior, which is available via a ‘context’ object. Signed-off-by: Stefan Marr <git@stefan-marr.de>
Signed-off-by: Stefan Marr <git@stefan-marr.de>
Signed-off-by: Stefan Marr <git@stefan-marr.de>
Signed-off-by: Stefan Marr <git@stefan-marr.de>
Signed-off-by: Stefan Marr <git@stefan-marr.de>
Signed-off-by: Stefan Marr <git@stefan-marr.de>
Class loader seems to be null on the bootclasspath Signed-off-by: Stefan Marr <git@stefan-marr.de>
Signed-off-by: Stefan Marr <git@stefan-marr.de>
@smarr ReflectiveCompiler is the latest dev branch. I could apply it if you prefer, but I do not know how to create a PR to another repo... |
This change adds basic infrastructure to use annotations to handle primitives in Truffle languages.
It supports parse-time and eager run-time specialization. During specialization, the normal method call can be replaced with a specific operation. This means, there is no overhead for a method call anymore in the interpreter.
How to use this code is shown for TruffleSOM in this PR: smarr/TruffleSOM#13
However, the code is still work-in-progress, because it is not yet directly useable for SOMns.
Open issues:
@charig @Richard-Roberts this is a preview. I'd be curious about what you think.
I just wanted to get things started and actually have some code going into the project.
If you look a the code, please check the JavaDoc, I tried to actually write some documentation.
VmSettings
This PR also introduces a basic framework for language-specific settings.
A language can implement the
Settings
interface.The name of the class implementing it, needs to be provided via the
bd.settings
Java property.From that property, the settings are loaded and used to initialize static fields of
VmSettings
.Interfaces
The PR now introduces the following interfaces:
EagerlySpecializable
, provides basic function needed to specialize a node eagerlyEagerPrimitive
, is used for nodes that replace the original function calls for instance and provides basic functions to execute the node correctly after replacing the old one in the ASTOperation
, is used by tools and marks a node to be an operation that has arguments and a namePreevaluatedExpression
provides a uniform method to execute a node with already evaluated subexpressionsWithContext
marks a node as requiring access to a language's context object, which typically holds some global state or behavior nodes might need access to.