Simplify floating point mod and round jit helpers implementations #100222
Conversation
|
Peeled from #98858 |
|
Tagging subscribers to this area: @mangod9 |
jkotas
changed the title
src/coreclr/vm/corelib.h
Outdated
|
|
||
| DEFINE_CLASS(MATHF, System, MathF) | ||
| DEFINE_METHOD(MATHF, ROUND, Round, SM_Flt_RetFlt) | ||
| DEFINE_METHOD(MATHF, FMOD, FMod, NoSig) |
There was a problem hiding this comment.
Can we also just remove Math.FMod
We can just replace any calls with simply x % y, since that does the same thing
There was a problem hiding this comment.
Can we also just remove
Math.FModWe can just replace any calls with simply
x % y, since that does the same thing
Won't this make porting FMod/% to a full managed impl harder later on?
Also FMod has some special intrinsic recognition in the JIT, not sure if % has the same today.
There was a problem hiding this comment.
Won't this make porting FMod/% to a full managed impl harder later on?
If FMod is ever ported to a fully managed impl, this can be adjusted as necessary.
Also FMod has some special intrinsic recognition in the JIT, not sure if % has the same today.
% has same value numbering support as FMod.
jkotas
force-pushed
the
math-helpers-floatingpoint
branch
from
23451ea to
0a04cb9
Compare
jkotas
requested review from
lambdageek,
thaystg and
marek-safar
as code owners
| break; | ||
| case ReadyToRunHelper.FltRem: | ||
| mangledName = "RhpFltRem"; | ||
| mangledName = "fmodf"; | ||
| break; | ||
|
|
There was a problem hiding this comment.
Just noticed that DblRound/FltRound are unhandled here (and on main), would it make sense to readd handling for them at least until they're fully removed?
There was a problem hiding this comment.
I am not sure what you mean by unhandled. Could you please elaborate?
There was a problem hiding this comment.
I am not sure what you mean by unhandled. Could you please elaborate?
There seems to be no switch case for them.
There was a problem hiding this comment.
| case ReadyToRunHelper.DblRound: | |
| DefType doubleType = context.GetWellKnownType(WellKnownType.Double); | |
| methodDesc = context.SystemModule.GetKnownType("System", "Math").GetKnownMethod("Round", | |
| new MethodSignature(MethodSignatureFlags.Static, 0, doubleType, [doubleType])); | |
| break; | |
| case ReadyToRunHelper.FltRound: | |
| DefType floatType = context.GetWellKnownType(WellKnownType.Single); | |
| methodDesc = context.SystemModule.GetKnownType("System", "MathF").GetKnownMethod("Round", | |
| new MethodSignature(MethodSignatureFlags.Static, 0, floatType, [floatType])); | |
| break; | |
if we want to add them here.
There was a problem hiding this comment.
This mapping is native AOT specific. It does not need to handle JIT helpers that are no longer used or that are only used outside native AOT.
jkotas
force-pushed
the
math-helpers-floatingpoint
branch
from
146a64d to
b269447
Compare
No description provided.