[NativeAOT-LLVM] Merge may24 #2591
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12131,12 +12131,14 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_ | |
|
|
||
| // for tracking down problems in reguse prediction or liveness tracking | ||
|
|
||
| #if !defined(TARGET_WASM) | ||
| if (verbose && 0) | ||
| { | ||
| printf(" RR="); | ||
| dspRegMask(JitTls::GetCompiler()->codeGen->internalRegisters.GetAll(tree)); | ||
| printf("\n"); | ||
| } | ||
| #endif //!TARGET_WASM | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7545,6 +7545,20 @@ bool Compiler::IsTargetIntrinsic(NamedIntrinsic intrinsicName) | |
|
|
||
| bool Compiler::IsIntrinsicImplementedByUserCall(NamedIntrinsic intrinsicName) | ||
| { | ||
| #if defined(TARGET_WASM) | ||
| // TODO-LLVM: Revisit when SIMD is enabled. | ||
| // We want the fallback LIR expansions for these. | ||
| switch(intrinsicName) | ||
| { | ||
| case NI_System_Math_MultiplyAddEstimate: | ||
| case NI_System_Math_ReciprocalEstimate: | ||
| case NI_System_Math_ReciprocalSqrtEstimate: | ||
| return false; | ||
| default: | ||
| break; | ||
| } | ||
| #endif //TARGET_WASM | ||
|
There was a problem hiding this comment. I have done some time thinking about this, and looking at dotnet/runtime#102098, and I think the better fix for this issue is to augment
The actual CQ item here is to enable the better CQ for these intrinsic by actually exploiting the non-determinism, but it's a bit hard because LLVM doesn't have the same notion of intrinsics deterministic on "process granularity". There was a problem hiding this comment. Changed public static double ReciprocalSqrtEstimate(double d)
{
#if MONO || TARGET_RISCV64 || TARGET_LOONGARCH64 || TARGET_WASM
return 1.0 / Sqrt(d);
#else
return ReciprocalSqrtEstimate(d);
#endif
}There was a problem hiding this comment. Why was it needed? It doesn't look expected to me. There was a problem hiding this comment. Its not needed, but I thought that' what you refered to in "requires a corresponding ...". I will revert it |
||
|
|
||
| // Currently, if a math intrinsic is not implemented by target-specific | ||
| // instructions, it will be implemented by a System.Math call. In the | ||
| // future, if we turn to implementing some of them with helper calls, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -810,22 +810,18 @@ CONFIG_INTEGER(JitSaveFpLrWithCalleeSavedRegisters, W("JitSaveFpLrWithCalleeSave | |
| CONFIG_INTEGER(JitDispIns, W("JitDispIns"), 0) | ||
| #endif // defined(TARGET_LOONGARCH64) | ||
|
|
||
| #ifdef TARGET_WASM | ||
| RELEASE_CONFIG_INTEGER(JitCheckLlvmIR, W("JitCheckLlvmIR"), 0) | ||
| RELEASE_CONFIG_INTEGER(JitRunLssaTests, W("JitRunLssaTests"), 0) | ||
| RELEASE_CONFIG_INTEGER(JitGcStress, W("JitGcStress"), 0) | ||
|
|
||
| #ifdef DEBUG | ||
| CONFIG_STRING(JitEnableLssaRange, W("JitEnableLssaRange")) | ||
| #endif // DEBUG | ||
yowl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| #endif // TARGET_WASM | ||
|
|
||
| // Allow to enregister locals with struct type. | ||
| RELEASE_CONFIG_INTEGER(JitEnregStructLocals, W("JitEnregStructLocals"), 1) | ||
|
|
||
| #undef CONFIG_INTEGER | ||
| #undef CONFIG_STRING | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1889,7 +1889,9 @@ void Compiler::lvaClassifyParameterABI() | |
| // first stack arg is not considered to be at offset 0. | ||
| // TODO-Cleanup: Unify things so that x86 is consistent with other platforms | ||
| // here and change fgMorphExpandStackArgForVarArgs to account for that. | ||
| // LLVM: staock offsets are not aligned in the classifier, if they are then this passes and | ||
| // assert(segment.Offset + segment.Size <= lvaLclExactSize(lclNum)) below fails. | ||
|
Comment on lines
+1892
to
+1893
There was a problem hiding this comment. Why does the second assert fail? There was a problem hiding this comment. Good question, I can change the classifier to if (type == TYP_STRUCT)
{
structPassingKind wbPassStruct;
type = comp->m_llvm->GetArgTypeForStructWasm(structLayout->GetClassHandle(), &wbPassStruct);
}
assert(type != TYP_STRUCT);
unsigned typeSize = genTypeSize(type);
ABIPassingSegment segment = ABIPassingSegment::OnStack(m_stackArgSize, 0, typeSize);
m_stackArgSize += AlignUp(typeSize, TARGET_POINTER_SIZE);
return ABIPassingInformation::FromSegment(comp, segment);But that doesn't match the values passed to There was a problem hiding this comment. There is an assumption in the compiler that We can delete this later - when the old classification is phased out. There was a problem hiding this comment. Thanks, Here the size is used as is, ignoring byref structs, and the value used to set the stack offset. However here https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/lclvars.cpp#L1904 the segment is checked against the size adusted for storing byrefs as pointers. How do we handle this to both pass this assert and the one that checks the classifer offsets with the stack offsets ? There was a problem hiding this comment. We would need to modify There was a problem hiding this comment. Ok, I left the |
||
| #if !defined(TARGET_X86) && !defined(TARGET_WASM) | ||
| assert(dscStackOffset == expected.GetStackOffset()); | ||
| #endif | ||
| } | ||
|
|
@@ -5947,6 +5949,7 @@ void Compiler::lvaUpdateArgsWithInitialReg() | |
| } | ||
| } | ||
|
|
||
| #if !defined(TARGET_WASM) | ||
| //----------------------------------------------------------------------------- | ||
| // lvaAssignVirtualFrameOffsetsToArgs: | ||
| // Assign virtual frame offsets to the incoming parameters. | ||
|
|
@@ -5970,25 +5973,12 @@ void Compiler::lvaAssignVirtualFrameOffsetsToArgs() | |
| relativeZero = genCountBits(prespilled) * TARGET_POINTER_SIZE; | ||
| #endif | ||
|
|
||
| for (unsigned lclNum = 0; lclNum < info.compArgsCount; lclNum++) | ||
| { | ||
| LclVarDsc* dsc = lvaGetDesc(lclNum); | ||
|
|
||
| int startOffset; | ||
| if (lvaGetRelativeOffsetToCallerAllocatedSpaceForParameter(lclNum, &startOffset)) | ||
| { | ||
| dsc->SetStackOffset(startOffset + relativeZero); | ||
| JITDUMP("Set V%02u to offset %d\n", lclNum, startOffset); | ||
|
|
@@ -6006,6 +5996,7 @@ void Compiler::lvaAssignVirtualFrameOffsetsToArgs() | |
| } | ||
| } | ||
| } | ||
| #endif // !TARGET_WASM | ||
|
|
||
| //----------------------------------------------------------------------------- | ||
| // lvaGetRelativeOffsetToCallerAllocatedSpaceForParameter: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -396,7 +396,7 @@ bool Llvm::helperCallMayPhysicallyThrow(CorInfoHelpFunc helperFunc) const | |
| { FUNC(CORINFO_HELP_ASSIGN_BYREF) }, // Not used on WASM. | ||
|
|
||
| // Not used in NativeAOT (or at all in some cases). | ||
| { FUNC(CORINFO_HELP_BULK_WRITEBARRIER) }, | ||
|
There was a problem hiding this comment. This one should go to the above group now with |
||
| { FUNC(CORINFO_HELP_GETFIELD8) }, | ||
| { FUNC(CORINFO_HELP_SETFIELD8) }, | ||
| { FUNC(CORINFO_HELP_GETFIELD16) }, | ||
|
|
@@ -615,7 +615,7 @@ CorInfoType Llvm::getLlvmArgTypeForArg(CorInfoType argSigType, CORINFO_CLASS_HAN | |
| } | ||
| // | ||
| // WASM C ABI is documented here: https://github.com/WebAssembly/tool-conventions/blob/main/BasicCABI.md. | ||
| // In essence, structs are passed by reference except if they are trivial wrappers of a primitive (scalar). | ||
| // We follow this rule for the native calling convention as well as the managed one. | ||
| // | ||
| bool isByRef = false; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should go upstream (probably the declarations should be ifdefed out too).