Support nocheck prefix

[john-h-k]

Resolves #10112

I would think that, at the very least, we should ignore the opcode (other than the "correctness/verifiability" checks) rather than throwing InvalidProgramException.

It is spec compliant, as it is just a hint, but right now it has no actual functionality (it is verified and immediately ignored). It needs test but I don't know how to write JIT tests yet

It does not allow multiple no. prefixes, such as
no. typecheck no. rangecheck ldelema - only no. typecheck | rangecheck ldelema

[ltrzesniewski]

I'd prefer if you didn't close the issue with a no-op implementation. Sure, it's ECMA-compliant and a good first step, but it does not really implement the feature I'm interested in. 😉

[john-h-k]

On a seperate branch I'm working on an actual functioning implementation. So far I've got the basic idea of range check elimination but it's hitting some assertions in morph

[john-h-k]

At the moment, GTF_INX_RNGCHK has the same value as GTF_VAR_DEF which is causing an assertion in gfMorphLocalVar to be hit

[abelbraaksma]

This is very interesting, and a language like F# could benefit greatly once this is implemented (meaning, if the nullcheck, rangecheck, typecheck are actually skipped). Esp the rangecheck is very beneficial, as in F# many code paths exist that are verifiably correct, but are not always optimized properly in JIT.

The nullcheck is already skipped quite often by JIT, though it wouldn't hurt making this explicit in F# code, which has a lot non-nullable types.

(I understand that, once this is implemented, such IL would not be verifiable itself, as the spec states.)

[john-h-k]

Came back to this and managed to get no. rangecheck ldelem/stelem, no. nullcheck callvirt and no. typecheck castclass all working. Some of the others require adding new helper methods which I'll give a shot tomorrow

[abelbraaksma]

Awesome stuff!

[AndyAyersMS]

@john-h-k this PR is getting a bit stale -- any plans to revive it, or should we close and revisit later?

[ViktorHofer]

// Auto-generated message

69e114c which was merged 12/7 removed the intermediate src/coreclr/src/ folder. This PR needs to be updated as it touches files in that directory which causes conflicts.

To update your commits you can use this bash script: https://gist.github.com/ViktorHofer/6d24f62abdcddb518b4966ead5ef3783. Feel free to use the comment section of the gist to improve the script for others.

[john-h-k]

@AndyAyersMS

Sorry about that - been very busy. I'll revisit it on Friday night or Saturday and give an update on it when I do

[AndyAyersMS]

@john-h-k seems like maybe we should close this?

[abelbraaksma]

I hope this could be resurrected, compilers could benefit greatly once this gets in.

[ghost]

Draft Pull Request was automatically closed for inactivity. It can be manually reopened in the next 30 days if the work resumes.