Skip to content

Handle large imm values in ARMEmitter.EmitLDR #98433

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.

Sign up for GitHub

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

Jump to bottom
Merged
jkotas merged 1 commit into from
Feb 14, 2024
Merged

Handle large imm values in ARMEmitter.EmitLDR #98433

jkotas merged 1 commit into from
Feb 14, 2024

Conversation

@filipnavara
Copy link
Member

@filipnavara filipnavara commented Feb 14, 2024

Fixes pri0 test failure:

2024-02-14T08:23:44.0403671Z     Process terminated. Assertion failed.
2024-02-14T08:23:44.0405627Z        at ILCompiler.DependencyAnalysis.ARM.ARMEmitter.EmitLDR(Register destination, Register source, Int32 offset) in /_/src/coreclr/tools/Common/Compiler/DependencyAnalysis/Target_ARM/ARMEmitter.cs:line 127
2024-02-14T08:23:44.0406283Z        at ILCompiler.DependencyAnalysis.ReadyToRunHelperNode.EmitCode(NodeFactory factory, ARMEmitter& encoder, Boolean relocsOnly) in /_/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMReadyToRunHelperNode.cs:line 36
2024-02-14T08:23:44.0406921Z        at ILCompiler.DependencyAnalysis.AssemblyStubNode.GetData(NodeFactory factory, Boolean relocsOnly) in /_/src/coreclr/tools/Common/Compiler/DependencyAnalysis/AssemblyStubNode.cs:line 59
2024-02-14T08:23:44.0407567Z        at ILCompiler.ObjectWriter.ObjectWriter.EmitObject(String objectFilePath, IReadOnlyCollection`1 nodes, IObjectDumper dumper, Logger logger) in /_/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs:line 391
2024-02-14T08:23:44.0420336Z        at ILCompiler.ObjectWriter.ObjectWriter.EmitObject(String objectFilePath, IReadOnlyCollection`1 nodes, NodeFactory factory, ObjectWritingOptions options, IObjectDumper dumper, Logger logger) in /_/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs:line 542
2024-02-14T08:23:44.0421003Z        at ILCompiler.Compilation.ILCompiler.ICompilation.Compile(String outputFile, ObjectDumper dumper) in /_/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs:line 525
2024-02-14T08:23:44.0421421Z        at ILCompiler.Program.Run() in /_/src/coreclr/tools/aot/ILCompiler/Program.cs:line 585
2024-02-14T08:23:44.0421751Z        at ILCompiler.ILCompilerRootCommand.<>c__DisplayClass236_0.<.ctor>b__0(ParseResult result) in /_/src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:line 292
2024-02-14T08:23:44.0422100Z        at System.CommandLine.Invocation.InvocationPipeline.Invoke(ParseResult parseResult)
2024-02-14T08:23:44.0422354Z        at System.CommandLine.ParseResult.Invoke()
2024-02-14T08:23:44.0422610Z        at ILCompiler.Program.Main(String[] args) in /_/src/coreclr/tools/aot/ILCompiler/Program.cs:line 753
2024-02-14T08:23:44.4226529Z /__w/1/s/artifacts/bin/coreclr/linux.arm.Checked/build/Microsoft.NETCore.Native.targets(310,5): error MSB3073: The command ""/__w/1/s/artifacts/bin/coreclr/linux.arm.Checked/x64/ilc/ilc" @"/__w/1/s/artifacts/tests/coreclr/obj/linux.arm.Checked/Managed/JIT/opt/virtualstubdispatch/bigvtbl/bigvtbl_cs_r/native/bigvtbl_cs_r.ilc.rsp"" exited with code 134.

Contributes to #97729

@filipnavara filipnavara added arch-arm32 community-contribution Indicates that the PR has been added by a community member area-NativeAOT-coreclr and removed area-crossgen2-coreclr labels Feb 14, 2024
@ghost
Copy link

ghost commented Feb 14, 2024

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes pri0 test failure:

2024-02-14T08:23:44.0403671Z     Process terminated. Assertion failed.
2024-02-14T08:23:44.0405627Z        at ILCompiler.DependencyAnalysis.ARM.ARMEmitter.EmitLDR(Register destination, Register source, Int32 offset) in /_/src/coreclr/tools/Common/Compiler/DependencyAnalysis/Target_ARM/ARMEmitter.cs:line 127
2024-02-14T08:23:44.0406283Z        at ILCompiler.DependencyAnalysis.ReadyToRunHelperNode.EmitCode(NodeFactory factory, ARMEmitter& encoder, Boolean relocsOnly) in /_/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMReadyToRunHelperNode.cs:line 36
2024-02-14T08:23:44.0406921Z        at ILCompiler.DependencyAnalysis.AssemblyStubNode.GetData(NodeFactory factory, Boolean relocsOnly) in /_/src/coreclr/tools/Common/Compiler/DependencyAnalysis/AssemblyStubNode.cs:line 59
2024-02-14T08:23:44.0407567Z        at ILCompiler.ObjectWriter.ObjectWriter.EmitObject(String objectFilePath, IReadOnlyCollection`1 nodes, IObjectDumper dumper, Logger logger) in /_/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs:line 391
2024-02-14T08:23:44.0420336Z        at ILCompiler.ObjectWriter.ObjectWriter.EmitObject(String objectFilePath, IReadOnlyCollection`1 nodes, NodeFactory factory, ObjectWritingOptions options, IObjectDumper dumper, Logger logger) in /_/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs:line 542
2024-02-14T08:23:44.0421003Z        at ILCompiler.Compilation.ILCompiler.ICompilation.Compile(String outputFile, ObjectDumper dumper) in /_/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs:line 525
2024-02-14T08:23:44.0421421Z        at ILCompiler.Program.Run() in /_/src/coreclr/tools/aot/ILCompiler/Program.cs:line 585
2024-02-14T08:23:44.0421751Z        at ILCompiler.ILCompilerRootCommand.<>c__DisplayClass236_0.<.ctor>b__0(ParseResult result) in /_/src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs:line 292
2024-02-14T08:23:44.0422100Z        at System.CommandLine.Invocation.InvocationPipeline.Invoke(ParseResult parseResult)
2024-02-14T08:23:44.0422354Z        at System.CommandLine.ParseResult.Invoke()
2024-02-14T08:23:44.0422610Z        at ILCompiler.Program.Main(String[] args) in /_/src/coreclr/tools/aot/ILCompiler/Program.cs:line 753
2024-02-14T08:23:44.4226529Z /__w/1/s/artifacts/bin/coreclr/linux.arm.Checked/build/Microsoft.NETCore.Native.targets(310,5): error MSB3073: The command ""/__w/1/s/artifacts/bin/coreclr/linux.arm.Checked/x64/ilc/ilc" @"/__w/1/s/artifacts/tests/coreclr/obj/linux.arm.Checked/Managed/JIT/opt/virtualstubdispatch/bigvtbl/bigvtbl_cs_r/native/bigvtbl_cs_r.ilc.rsp"" exited with code 134.

Contributes to #97729

Author: filipnavara
Assignees: -
Labels:

arch-arm32, community-contribution, area-NativeAOT-coreclr
Milestone: -

filipnavara
filipnavara commented Feb 14, 2024
View reviewed changes
src/coreclr/tools/Common/Compiler/DependencyAnalysis/Target_ARM/ARMEmitter.cs
Comment on lines +134 to +148
uint constVal = (uint)offset & ~0xfffu;
uint mask32 = 0xff;
uint imm8 = 0;
int encode = 31; // 11111

do
{
mask32 <<= 1;
if ((constVal & ~mask32) == 0)
{
imm8 = (constVal & mask32) >> (32 - encode);
break;
}
encode--;
} while (encode >= 8);
Copy link
Member Author

@filipnavara filipnavara Feb 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case you are wondering, this part is shamelessly copied from the JIT:

runtime/src/coreclr/jit/emitarm.cpp

Lines 1349 to 1366 in c7c15f7

mask32 = 0x00000ff;
encode = 31; /* 11111 */
do
{
mask32 <<= 1;
temp = uval32 & ~mask32;
if (temp == 0)
{
imm8 = (uval32 & mask32) >> (32 - encode);
assert((imm8 & 0x80) != 0);
goto DONE;
}
encode--;
} while (encode >= 8);
assert(!"encodeModImmConst failed!");
return BAD_CODE;

The instruction encoding has several forms. We want to use the one where we produce an immediate value and rotate it using the barrel shifter into the right position to get the right constant.

Copy link
Member

@jkotas jkotas Feb 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not look like that it can.

jkotas
jkotas reviewed Feb 14, 2024
View reviewed changes
src/coreclr/tools/Common/Compiler/DependencyAnalysis/Target_ARM/ARMEmitter.cs
{
Debug.Assert(IsValidReg(destination) && IsValidReg(source));

if (offset >= 0x1000)
Copy link
Member

@jkotas jkotas Feb 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be ever called with large negative offset?

Copy link
Member Author

@filipnavara filipnavara Feb 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the call sites and the only negative values seemed to be in the -PointerSize range.

jkotas
jkotas approved these changes Feb 14, 2024
View reviewed changes
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@jkotas jkotas merged commit b2e93f8 into dotnet:main Feb 14, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 16, 2024
@filipnavara filipnavara deleted the ldr-long-imm branch June 5, 2025 07:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jkotas jkotas jkotas approved these changes

@MichalStrehovsky MichalStrehovsky Awaiting requested review from MichalStrehovsky MichalStrehovsky is a code owner

Assignees

No one assigned

Labels

arch-arm32 area-NativeAOT-coreclr community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@filipnavara @jkotas