-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Description
Today, the Jit supports folding casts like these: (uint)float.MaxValue. The value returned by this cast when done by the Jit is defined by the C++ compiler it was compiled with, which may or may not match the value returned by the instruction that will be emitted for an equivalent dynamic cast. It also may or may not match the value Roslyn will produce (0) when constant-folding these casts at compile time.
One of the manifestations of this issue was observed in #47374, where MSVC x86 compiler gave a different answer to a question that would "normally" return 0.
However, a scenario like the above is actually not the main concern that I would like to raise for consideration in this issue. A much bigger problem is cross-compilation, which, presumably, will become fairly common once Crossgen2 is fully productized, where the AOT Jit can potentially give a different answer to the same question than the JIT Jit.
As I see it, there are a couple of options that can pursued here:
-
Do nothing. As per the ECMA specification, overflowing casts from floating-point types to integers are undefined behavior. This could lead to some problems (described below), but hopefully the impact will be minimal as such casts are not exactly widespread, and only an incredibly small fraction of them will involve constants (I conjecture this can only occur in real-world code when inlining).
-
Do not fold when the result is undefined (as per ECMA), neither in AOT, nor the JIT mode. This options has a couple of benefits associated with it:
- Behavior of folded code and non-folded code will be consistent for one architecture. This is in particular means that Release and Debug build should behave the same.
- AOT mode will be consistent with the JIT mode "for free", as no implementation-defined folding will be done at AOT time.
- It is a relatively simple approach, implementation wise.
However, this would mean that some folding opportunities will be missed and some UB not taken advantage of when optimizing, which can have cascading effects on, for example, inlining.
-
Only fold when not in AOT mode. This will eliminate the cross-compilation concerns, but can still potentially results in inconsistencies, depending on how the folding will be implemented (see below).
-
Fold always. This will be a stronger option than the above, and have the same problems/benefits, just magnified by the amount of code that will be exposed.
Now, one critical decision to make if it is to be decided that the folding is to be performed for these edge cases, is how should it be done? Again, there are a couple of options:
- Fold everything to zero. This is what Roslyn does today, but it would mean that there will be inconsistencies between the folding and the dynamic execution, which existing comments in the code highlight as undesirable:
runtime/src/coreclr/jit/gentree.cpp
Lines 14310 to 14313 in fb001e6
// to an integer, ..., the value returned is unspecified." However, it would // at least be desirable to have the same value returned for casting an overflowing // constant to an int as would obtained by passing that constant as a parameter // then casting that parameter to an int type. We will assume that the C compiler's
This is the reason why e. g.(uint)float.PositiveInfinityis not folded today, as are conversions fromNaN. Opting for this behavior is also simple in terms of the implementation, but it comes with the usual baggage that UB has: Debug vs Release inconsistencies, inlining vs no inlining inconsistencies (which may become dynamic with the advent of PGO and such), etc. - Fold to the same value the native instruction of the target architecture doing the conversion would produce when casting dynamically. This means that for
x64, for example, the Jit would followvcvttss2si's behavior, for ARM64 -fcvtzus, and so on. This is a relatively expensive option implementation-wise as the existing codegen would need to be surveyed for the exact instructions used, their behavior determined and stored in a dedicated table, which would need to be kept in sync with any future changes in the code generator. Hopefully, this should be a fixed-sized cost however, as it is not really expected that the emitted instructions will be changing frequently, if at all.
As I am no expert of the matter, this is only an overview of the problem, with the detailed analysis on the cost/benefit ratios of the options missing, with the intent on starting the discussion and determining those concretely.
It may turn out that the decision that will conclude this is that this simply is a non-issue extremely unlikely to be hit by anyone in the real world and thus doesn't merit any effort being dedicated to fixing it.
category:design
theme:optimization
Metadata
Metadata
Assignees
Labels
Type
Projects
Status