-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JIT: Unspill normalize-on-load variables using exact small type #90318
Merged
Merged
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
e405e94
JIT: Unspill normalize-on-load variables using exact small type
jakobbotsch 15be0fe
Add regression test
jakobbotsch 9c59fe8
Fix logic, update comment
jakobbotsch d6d0e08
Disable test on Mono
jakobbotsch b4b6654
Update src/coreclr/jit/codegenlinear.cpp
jakobbotsch ab21c79
Address feedback; scope down
jakobbotsch a7c4210
Run jit-format
jakobbotsch 803ac86
Update src/coreclr/jit/compiler.hpp
jakobbotsch File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -1211,39 +1211,36 @@ void CodeGen::genUnspillRegIfNeeded(GenTree* tree) | |||||
// Reset spilled flag, since we are going to load a local variable from its home location. | ||||||
unspillTree->gtFlags &= ~GTF_SPILLED; | ||||||
|
||||||
GenTreeLclVar* lcl = unspillTree->AsLclVar(); | ||||||
LclVarDsc* varDsc = compiler->lvaGetDesc(lcl); | ||||||
var_types spillType = varDsc->GetRegisterType(lcl); | ||||||
assert(spillType != TYP_UNDEF); | ||||||
|
||||||
// TODO-Cleanup: The following code could probably be further merged and cleaned up. | ||||||
#if defined(TARGET_XARCH) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) | ||||||
// Load local variable from its home location. | ||||||
// Never allow truncating the locals here, otherwise a subsequent | ||||||
// use of the local with a wider type would see the truncated | ||||||
// value. We do allow wider loads as those can be efficient even | ||||||
// when unaligned and might be smaller encoding wise (on xarch). | ||||||
var_types lclLoadType = varDsc->lvNormalizeOnLoad() ? varDsc->TypeGet() : varDsc->GetStackSlotHomeType(); | ||||||
assert(lclLoadType != TYP_UNDEF); | ||||||
if (genTypeSize(spillType) < genTypeSize(lclLoadType)) | ||||||
{ | ||||||
spillType = lclLoadType; | ||||||
} | ||||||
GenTreeLclVar* lcl = unspillTree->AsLclVar(); | ||||||
LclVarDsc* varDsc = compiler->lvaGetDesc(lcl); | ||||||
|
||||||
// Pick type to reload register from stack with. Note that in | ||||||
// general, the type of 'lcl' does not have any relation to the | ||||||
// type of 'varDsc': | ||||||
// | ||||||
// * For NOL locals it is wider under normal circumstances, where | ||||||
// morph has added a cast on top | ||||||
// * For all locals it can be narrower narrower in some cases, e.g. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
// when lowering optimizes to use a smaller typed `cmp` (e.g. | ||||||
// 32-bit cmp for 64-bit local, or 8-bit cmp for 16-bit local). | ||||||
// | ||||||
// Thus, we should always pick the type from varDsc. For NOL locals | ||||||
// we further want to make sure we always leave a normalized value | ||||||
// in the register. | ||||||
|
||||||
var_types unspillType = varDsc->lvNormalizeOnLoad() ? varDsc->TypeGet() : varDsc->GetStackSlotHomeType(); | ||||||
assert(unspillType != TYP_UNDEF); | ||||||
|
||||||
#if defined(TARGET_LOONGARCH64) | ||||||
if (varTypeIsFloating(spillType) && emitter::isGeneralRegister(tree->GetRegNum())) | ||||||
{ | ||||||
spillType = spillType == TYP_FLOAT ? TYP_INT : TYP_LONG; | ||||||
jakobbotsch marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} | ||||||
#endif | ||||||
#elif defined(TARGET_ARM) | ||||||
// No normalizing for ARM | ||||||
#else | ||||||
NYI("Unspilling not implemented for this target architecture."); | ||||||
#endif | ||||||
|
||||||
bool reSpill = ((unspillTree->gtFlags & GTF_SPILL) != 0); | ||||||
bool isLastUse = lcl->IsLastUse(0); | ||||||
genUnspillLocal(lcl->GetLclNum(), spillType, lcl->AsLclVar(), tree->GetRegNum(), reSpill, isLastUse); | ||||||
genUnspillLocal(lcl->GetLclNum(), unspillType, lcl->AsLclVar(), tree->GetRegNum(), reSpill, isLastUse); | ||||||
} | ||||||
else if (unspillTree->IsMultiRegLclVar()) | ||||||
{ | ||||||
|
110 changes: 110 additions & 0 deletions
110
src/tests/JIT/Regression/JitBlue/Runtime_90219/Runtime_90219.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using Xunit; | ||
|
||
// Generated by Fuzzlyn v1.5 on 2023-08-05 16:08:41 | ||
// Run on X86 Windows | ||
// Seed: 7610693270284065118 | ||
// Reduced from 12.0 KiB to 2.2 KiB in 00:01:06 | ||
// Debug: Outputs 1 | ||
// Release: Outputs 2 | ||
public interface I0 | ||
{ | ||
} | ||
|
||
public class C0 : I0 | ||
{ | ||
public ulong F0; | ||
public uint F4; | ||
public C0(ulong f0, uint f4) | ||
{ | ||
F0 = f0; | ||
F4 = f4; | ||
} | ||
} | ||
|
||
public class Runtime_90219 | ||
{ | ||
public static IRuntime s_rt; | ||
public static I0 s_1; | ||
public static uint s_2; | ||
public static byte[] s_4 = new byte[]{0}; | ||
public static byte Result; | ||
|
||
[Fact] | ||
public static int TestEntryPoint() | ||
{ | ||
CollectibleALC alc = new CollectibleALC(); | ||
System.Reflection.Assembly asm = alc.LoadFromAssemblyPath(System.Reflection.Assembly.GetExecutingAssembly().Location); | ||
System.Reflection.MethodInfo mi = asm.GetType(typeof(Runtime_90219).FullName).GetMethod(nameof(MainInner)); | ||
System.Type runtimeTy = asm.GetType(typeof(Runtime).FullName); | ||
return (int)mi.Invoke(null, new object[]{System.Activator.CreateInstance(runtimeTy)}); | ||
} | ||
|
||
public static int MainInner(IRuntime rt) | ||
{ | ||
s_rt = rt; | ||
var vr8 = new C0(0, 0); | ||
var vr12 = s_4[0]; | ||
vr8.F0 *= M2(vr12); | ||
long[] vr9 = new long[]{0}; | ||
bool vr10 = (int)M2(0) <= vr9[0]; | ||
return Result == 1 ? 100 : 101; | ||
} | ||
|
||
public static ulong M2(byte arg0) | ||
{ | ||
for (int var0 = 0; var0 < 2; var0++) | ||
{ | ||
var vr1 = new C0(0, 0); | ||
var vr3 = new C0(0, 0); | ||
uint vr14 = s_2; | ||
s_rt.WriteLine("c_0", vr14); | ||
uint vr15 = vr3.F4; | ||
s_2 = M3(vr1, vr15); | ||
s_1 = s_1; | ||
s_1 = new C0(0, 0); | ||
s_rt.WriteLine("c_6", var0); | ||
} | ||
|
||
var vr5 = new C0(0, 1); | ||
uint vr18 = vr5.F4; | ||
arg0 = (byte)vr18; | ||
var vr7 = new C0(0, 1838341904U); | ||
if ((arg0 >= (byte)M3(vr7, 0))) | ||
{ | ||
arg0 <<= 1; | ||
} | ||
|
||
s_rt.WriteLine("c_7", arg0); | ||
return 0; | ||
} | ||
|
||
public static uint M3(C0 argThis, uint arg0) | ||
{ | ||
s_rt.WriteLine("c_0", arg0); | ||
return argThis.F4; | ||
} | ||
} | ||
|
||
public interface IRuntime | ||
{ | ||
void WriteLine<T>(string site, T value); | ||
} | ||
|
||
public class Runtime : IRuntime | ||
{ | ||
public void WriteLine<T>(string site, T value) | ||
{ | ||
if (typeof(T) == typeof(byte)) | ||
Runtime_90219.Result = (byte)(object)value; | ||
} | ||
} | ||
|
||
public class CollectibleALC : System.Runtime.Loader.AssemblyLoadContext | ||
{ | ||
public CollectibleALC(): base(true) | ||
{ | ||
} | ||
} |
8 changes: 8 additions & 0 deletions
8
src/tests/JIT/Regression/JitBlue/Runtime_90219/Runtime_90219.csproj
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
<PropertyGroup> | ||
<Optimize>True</Optimize> | ||
</PropertyGroup> | ||
<ItemGroup> | ||
<Compile Include="$(MSBuildProjectName).cs" /> | ||
</ItemGroup> | ||
</Project> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
nit: spell out NOL the first time