Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 2 additions & 32 deletions src/coreclr/jit/emit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8516,40 +8516,10 @@ void emitter::emitOutputDataSec(dataSecDsc* sec, AllocMemChunk* chunks)

if (m_compiler->opts.compReloc)
{
#ifdef TARGET_ARM
// The runtime and ILC will handle setting the thumb bit on the async resumption stub entrypoint,
// either directly in the emitAsyncResumeStubEntryPoint value (runtime) or will add the thumb bit
// to the symbol definition (ilc). ReadyToRun is different here: it emits method symbols without the
// thumb bit, then during fixups, the runtime adds the thumb bit. This works for all cases where
// the method entrypoint is fixed up at runtime, but doesn't hold for the resumption stub, which is
// emitted as a direct call without the typical indirection cell + fixup. This is okay in this case
// (while regular method calls could not do this) because the async method and its resumption stub
// are tightly coupled and effectively funclets of the same method. However, this means that
// crossgen needs the reloc for the resumption stubs entrypoint to include the thumb bit. Until we
// unify the behavior of crossgen with the runtime and ilc, we will work around this by emitting the
// reloc with the addend for the thumb bit.
if (m_compiler->IsReadyToRun())
{
emitRecordRelocationWithAddlDelta(&aDstRW[i].Resume, emitAsyncResumeStubEntryPoint,
CorInfoReloc::DIRECT, 1);
}
else
#endif
{
emitRecordRelocation(&aDstRW[i].Resume, emitAsyncResumeStubEntryPoint, CorInfoReloc::DIRECT);
}
emitRecordRelocation(&aDstRW[i].Resume, emitAsyncResumeStubEntryPoint, CorInfoReloc::DIRECT);
if (target != nullptr)
{
#ifdef TARGET_ARM
if (m_compiler->IsReadyToRun())
{
emitRecordRelocationWithAddlDelta(&aDstRW[i].DiagnosticIP, target, CorInfoReloc::DIRECT, 1);
}
else
#endif
{
emitRecordRelocation(&aDstRW[i].DiagnosticIP, target, CorInfoReloc::DIRECT);
}
emitRecordRelocation(&aDstRW[i].DiagnosticIP, target, CorInfoReloc::DIRECT);
}
}

Expand Down
7 changes: 4 additions & 3 deletions src/coreclr/jit/emitarm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5307,9 +5307,10 @@ BYTE* emitter::emitOutputLJ(insGroup* ig, BYTE* dst, instrDesc* i)
assert(ins == INS_movw || ins == INS_movt);
distVal = (ssize_t)emitOffsetToPtr(dstOffs);

// ILC defines method symbols with the thumb bit already set, so don't add it here.
// For ReadyToRun and non-relocatable code (runtime JIT), we set it ourselves.
if (!m_compiler->IsNativeAot())
// ILC and crossgen2 defines method symbols with the thumb bit already set, so don't add it here.
Comment thread
jtschuster marked this conversation as resolved.
// Assume compilations with relocs will put the thumb bit in the symbol.
// For non-relocatable code (runtime JIT), we set it ourselves.
if (!(m_compiler->opts.compReloc))
Comment thread
jtschuster marked this conversation as resolved.
{
distVal += 1;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ or IMAGE_REL_BASED_THUMB_BRANCH24 or IMAGE_REL_BASED_THUMB_MOV32_PCREL &&
// Method symbols should be defined with the thumb bit (+1) set per the AAELF ABI
// convention. For BRANCH24, the encoding cannot represent the thumb bit
// (per AAELF formula ((S + A) | T) – P), so strip it from the symbol value.
// NOTE: R2R doesn't currently add the thumb bit to the symbol value, so this is a NOP.
long symbolValue = relocType is IMAGE_REL_BASED_THUMB_BRANCH24
? definedSymbol.Value & ~1L
: definedSymbol.Value;
Expand Down Expand Up @@ -409,12 +408,8 @@ public virtual void EmitObject(Stream outputFileStream, IReadOnlyCollection<Depe
}

bool isMethod = node is IMethodBodyNode or AssemblyStubNode;
#if !READYTORUN
long thumbBit = _nodeFactory.Target.Architecture == TargetArchitecture.ARM && isMethod ? 1 : 0;
#else
// R2R records the thumb bit in the addend when needed, so we don't have to do it here.
long thumbBit = 0;
#endif

if (node is WasmTypeNode signature)
{
RecordMethodSignature(signature);
Expand Down
11 changes: 10 additions & 1 deletion src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4329,9 +4329,18 @@ private void recordRelocation(void* location, void* locationRW, void* target, Co
break;
}

RelocType relocType = GetRelocType(fRelocType);
relocDelta += addlDelta;

RelocType relocType = GetRelocType(fRelocType);
if (_compilation.TypeSystemContext.Target.Architecture == TargetArchitecture.ARM &&
relocType == RelocType.IMAGE_REL_BASED_HIGHLOW &&
targetBlock == BlockType.Code)
{
// The ARM JIT reports PCode targets with the Thumb bit set. Method symbols also
// carry the Thumb bit, so keep the relocation addend as an offset from the code byte.
Comment on lines +4339 to +4340
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This sounds like there's one more spot in RyuJIT like the one this PR is touching in emitarm.cpp that is adding the thumb bit when it shouldn't. A bit odd we were not running into it with native AOT.

relocDelta -= _compilation.TypeSystemContext.Target.CodeDelta;
}

// relocDelta is stored as the value
Relocation.WriteValue(relocType, location, relocDelta);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

<ItemGroup>
<ProjectReference Include="../ILCompiler.Reflection.ReadyToRun/ILCompiler.Reflection.ReadyToRun.csproj" />
<ProjectReference Include="../crossgen2/crossgen2_inbuild.csproj" ReferenceOutputAssembly="false" />
</ItemGroup>

<!-- Test case source files are embedded as resources so we can compile them with Roslyn at test time.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Runtime.CompilerServices;

public static class ExceptionHandling
{
public static int CallDependency() => InlineableLib.GetValue() + MethodWithSwitchTable(5);

public static int MethodWithExceptionInfo(int value)
{
try
{
return 100 / value;
}
catch (DivideByZeroException)
{
return -1;
}
}

public static int MethodWithSwitchTable(int value)
{
return value switch
{
0 => Case0(),
1 => Case1(),
2 => Case2(),
3 => Case3(),
4 => Case4(),
5 => Case5(),
6 => Case6(),
_ => CaseDefault(),
};
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static int Case0() => 10;

[MethodImpl(MethodImplOptions.NoInlining)]
private static int Case1() => 11;

[MethodImpl(MethodImplOptions.NoInlining)]
private static int Case2() => 12;

[MethodImpl(MethodImplOptions.NoInlining)]
private static int Case3() => 13;

[MethodImpl(MethodImplOptions.NoInlining)]
private static int Case4() => 14;

[MethodImpl(MethodImplOptions.NoInlining)]
private static int Case5() => 15;

[MethodImpl(MethodImplOptions.NoInlining)]
private static int Case6() => 16;

[MethodImpl(MethodImplOptions.NoInlining)]
private static int CaseDefault() => 17;
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,42 @@ static void Validate(ReadyToRunReader reader)
}
}

[Fact]
public void ArmThumbBitRelocationTargets()
{
var inlineableLib = new CompiledAssembly
{
AssemblyName = "InlineableLib",
SourceResourceNames = ["CrossModuleInlining/Dependencies/InlineableLib.cs"],
};
var exceptionHandling = new CompiledAssembly
{
AssemblyName = nameof(ArmThumbBitRelocationTargets),
SourceResourceNames = ["CrossModuleInlining/ExceptionHandling.cs"],
References = [inlineableLib],
};

new R2RTestRunner(_output).Run(new R2RTestCase(
nameof(ArmThumbBitRelocationTargets),
[
new(nameof(ArmThumbBitRelocationTargets),
[
new CrossgenAssembly(exceptionHandling),
new CrossgenAssembly(inlineableLib) { Kind = Crossgen2InputKind.Reference },
])
{
Options = [Crossgen2Option.TargetArchArm],
Validate = Validate,
},
]));

static void Validate(ReadyToRunReader reader)
{
string diag;
Assert.True(R2RAssert.HasExpectedArmThumbBitTargets(reader, out diag), diag);
}
}

[Fact]
public void TransitiveReferences()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ internal enum Crossgen2Option
ObjectFormat,
HotColdSplitting,
Optimize,
TargetArch,
TargetOS,
TargetArchArm,
}

internal static class Crossgen2OptionsExtensions
Expand All @@ -61,8 +60,7 @@ internal static class Crossgen2OptionsExtensions
Crossgen2Option.ObjectFormat => $"--object-format",
Crossgen2Option.HotColdSplitting => $"--hot-cold-splitting",
Crossgen2Option.Optimize => $"--optimize",
Crossgen2Option.TargetArch => $"--target-arch",
Crossgen2Option.TargetOS => $"--target-os",
Crossgen2Option.TargetArchArm => $"--targetarch:arm",
Comment thread
jtschuster marked this conversation as resolved.
_ => throw new ArgumentOutOfRangeException(nameof(kind)),
};
}
Expand Down
Loading
Loading