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

Add Tier0 jit flag #10580

Merged
merged 1 commit into from
Apr 4, 2017
Merged

Add Tier0 jit flag #10580

merged 1 commit into from
Apr 4, 2017

Conversation

noahfalk
Copy link
Member

This flag provides a hook to change the JIT policy in the future and diverge tier0 compilation from the min_opt policy.

This is a follow up change discussed in #10478

We can replace this flag as needed with even more flexible settings mechanisms, but this is a simple place to start.

@noahfalk
Copy link
Member Author

@AndyAyersMS
Copy link
Member

I'd rather see this a distinct flag value in corjit.h with the "mapping" onto minopts done on the jit side over in compInitOptions. That way it is simpler to experiment with new behaviors.

@JosephTremoulet
Copy link

I'd rather see this a distinct flag value in corjit.h with the "mapping" onto minopts done on the jit side over in compInitOptions. That way it is simpler to experiment with new behaviors.

I think it could be useful to have three different views and two mappings:

  • The code to re-jit can just say "this is Tier0", like this change
  • The cor jit flags communicate goals from the VM to the JIT (so Tier0 would map to a FastJit cor jit flag which we don't have yet, the intent of #10560 was to figure out what's the right set there)
  • The jit side would then map the interface flags to the set of optimizations and heuristic knob settings that it wants to run

(and extrapolating from that, I think it would make sense e.g. to move the "use minopts for cctors" rule to the VM side...)

Or am I over-thinking it and the interface should just be the current flags plus a bit to set on tier 0?

@AndyAyersMS
Copy link
Member

I'd like to see a discretionary policy decisions happen in one place if possible, which likely means they should happen in the jit. If the VM knows something about a method that entails certain codegen, then it can ask for it specifically. But if it is up to the jit what kind of code is appropriate, then let's try and put all that logic in one place, and have the VM pass in whatever extra information it has to the jit, so the jit can make reasonable choices.

@noahfalk
Copy link
Member Author

I'm happy to do whatever works for you guys. @JosephTremoulet if you agree with @AndyAyersMS let me know and I'll make the update. If you want to continue discussing I'll wait to make further changes until an agreement has been reached. No rush at my end.

@JosephTremoulet
Copy link

I'd like to see a discretionary policy decisions happen in one place if possible

That sounds reasonable. Extrapolating from that, then, it seems like

  • When the EE passes CORJIT_FLAG_MIN_OPT to the jit, this really simply means "the NoOptimization flag was set in the method's metadata (or COMPLUS var)" (this seems to be how the VM sets it today)
  • We could get rid of CORJIT_FLAG_SIZE_OPT and CORJIT_FLAG_SIZE_OPT (these seem to only be set for COMPLUS vars?)
  • The JIT should be able to distinguish single-shot jit vs tier 0 vs tier 1 -- this could be two new flags, or (maybe?) some combination of one flag and #ifdef FEATURE_TIERED_COMPILATION
  • The JIT should really/eventually be informing the TieredCompilationManager what conditions should prompt re-jitting at the next tier (presumably by compiling checks and conditional calls to a "rejit this" helper into the jitted code), though that raises some interesting questions about what conditions to bake into a method being compiled AOT...

Does that all sound right?

@noahfalk
Copy link
Member Author

Thanks Joe!

•The JIT should really/eventually be informing the TieredCompilationManager what conditions should prompt re-jitting at the next tier (presumably by compiling checks and conditional calls to a "rejit this" helper into the jitted code), though that raises some interesting questions about what conditions to bake into a method being compiled AOT...

I think it is very reasonable for arbitrary jit instrumentation to be a trigger for tiered compilation, but it is not something I am trying to build in the short term. I think it will be something we want longer term and I'll avoid doing anything that would make it harder to achieve when the time comes.

•The JIT should be able to distinguish single-shot jit vs tier 0 vs tier 1 -- this could be two new flags, or (maybe?) some combination of one flag and #ifdef FEATURE_TIERED_COMPILATION

If you want single-shot to potentially be different than tier 1 we should have a distinct flag. Even when FEATURE_TIERED_COMPILATION is on, there are still going to be methods that we determine can't be tiered for various reasons.

•We could get rid of CORJIT_FLAG_SIZE_OPT and CORJIT_FLAG_SIZE_OPT (these seem to only be set for COMPLUS vars?)

I'm not opposed at all to the principle, but just for logistics I'm going to leave my change scoped to work that is immediately relevant for tiered jitting.

@noahfalk noahfalk force-pushed the fitjit_tier0_flag branch from b6086d9 to 437177f Compare March 31, 2017 10:06
@JosephTremoulet
Copy link

LGTM (assuming formatting fix), except I'd expect we should update the JIE/EE interface guid

@AndyAyersMS
Copy link
Member

LGTM too.

@noahfalk
Copy link
Member Author

noahfalk commented Apr 1, 2017

assuming formatting fix

Did I miss a comment somewhere? I'm not sure what you are refering to.

@AndyAyersMS
Copy link
Member

There are formatting legs failing in CI. Details should be there in the logs.

You can:

  • fix by hand, or
  • apply the patches from the CI, or
  • clone the dotnet/jitutils repo and run the jit-format tool in fix mode

@noahfalk
Copy link
Member Author

noahfalk commented Apr 1, 2017

Ah thanks! I checked the logs and all makes sense once again.

These flags provides a hook to change the JIT policy in the future and diverge tier0/tier1 compilation from min_opt/speed_opt respectively.
@noahfalk noahfalk force-pushed the fitjit_tier0_flag branch from 437177f to 4561dad Compare April 1, 2017 02:07
@noahfalk
Copy link
Member Author

noahfalk commented Apr 4, 2017

@dotnet-bot test Tizen armel Cross Debug Build

@noahfalk noahfalk merged commit 4c1df4b into dotnet:master Apr 4, 2017
@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