Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Allow FEATURE_INTERPRETER to be enabled by command-line option #11188

Merged
merged 2 commits into from
Apr 25, 2017

Conversation

parjong
Copy link

@parjong parjong commented Apr 24, 2017

No description provided.

@BruceForstall
Copy link
Member

Who is going to define the cmake FEATURE_INTERPRETER variable? What platforms/configurations do you expect to enable the interpreter?

@BruceForstall
Copy link
Member

cc @dotnet/jit-contrib

@parjong
Copy link
Author

parjong commented Apr 25, 2017

@BruceForstall I would like to take a look it just for fun.

@parjong
Copy link
Author

parjong commented Apr 25, 2017

To be more specific, I recently read this article, and have interested in the interpreter internal.

@BruceForstall
Copy link
Member

Presumably you could just add the

add_definitions(-DFEATURE_INTERPRETER)

in a private branch. Why do you need to merge this change? I'm no cmake expert: how does merging this change make it easier?

Bottom-line: I don't want there to be any chance the interpreter is accidentally compiled in by default.

@parjong
Copy link
Author

parjong commented Apr 25, 2017

This PR allows users to enable FEATURE_INTERPERTER using the following command-line:

$ ./build.sh cmakeargs "-DFEATURE_INTERPRETER=1" ...

I want to build and test this feature without additional patch, but I also do not want to turn it default. I updated PR not to turn on this feature accidentally. FEATURE_INTERPRETER will be turned on only when users provide the additional command-line option mentioned above.

@BruceForstall BruceForstall merged commit c425377 into dotnet:master Apr 25, 2017
@parjong parjong deleted the fix/FEATURE_INTERPRETER branch April 26, 2017 01:37
@mattwarren
Copy link

mattwarren commented Apr 26, 2017

@parjong glad your found the blog post useful, it was fun to write!

BTW if you want to get all the statistics printed out at the end of a program run, you also need to make the following changes in interpreter.h, I didn't cover that in the post

// If this is set, we keep track of extra information about IL instructions executed per-method.
-#define INTERP_PROFILE 0
+#define INTERP_PROFILE 1

// If this is set, we track the distribution of IL instructions.
-#define INTERP_ILINSTR_PROFILE 0
+#define INTERP_ILINSTR_PROFILE 1

@parjong
Copy link
Author

parjong commented Apr 26, 2017

@mattwarren Thanks you for comment 👍 I'll try.

@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants