Skip to content

Conversation

@dawedawe
Copy link
Contributor

@dawedawe dawedawe commented Jun 14, 2023

This is an alternative approach to #15260 (RFC1011)
Instead of operating in PostInferenceChecks.fs this PR is using the IL level to emit warnings.
It seems a lot easier to emit warnings from there, but this approach currently suffers from a lot of false positives as can be seen in the tests.

  • Identify calls from outside the rec def, so we don't emit false positives
  • Identify rewrites to loops, so we don't emit false positives

@vzarytovskii
Copy link
Member

vzarytovskii commented Jun 19, 2023

Haven't reviewed it in detail yet, however, the first thouhg is that ilxgen doesn't feel like the right place for doing the tailcall analysis.

The analysis happens when we emit call(virt)s, which is orthogonal to the F#'s (tail) recursion (when we unroll recursion to loops, etc).

@dawedawe
Copy link
Contributor Author

Haven't reviewed it in detail yet, however, the first thouhg is that ilxgen doesn't feel like the right place for doing the tailcall analysis.

The analysis happens when we emit call(virt)s, which is orthogonal to the F#'s (tail) recursion (when we unroll recursion to loops, etc).

@vzarytovskii Thanks for your thoughts on this.
Yeah, I'm struggling a bit with finding the right place which provides the needed information.
Can you point me into some direction? Should it all be done in the optimizer?

@vzarytovskii
Copy link
Member

Haven't reviewed it in detail yet, however, the first thouhg is that ilxgen doesn't feel like the right place for doing the tailcall analysis.
The analysis happens when we emit call(virt)s, which is orthogonal to the F#'s (tail) recursion (when we unroll recursion to loops, etc).

@vzarytovskii Thanks for your thoughts on this. Yeah, I'm struggling a bit with finding the right place which provides the needed information. Can you point me into some direction? Should it all be done in the optimizer?

That's a good question, I would think that post inference phase, or optimization phase are the most fitting ones.

@dawedawe
Copy link
Contributor Author

dawedawe commented Jul 7, 2023

For those following along, I thing this PR is superseded by #15503
Please go there for the lastest work.

@dawedawe dawedawe closed this Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants