Skip to content

Commit

Permalink
NativeAOT/win/arm64: Fix the reloc type typo (dotnet#104516)
Browse files Browse the repository at this point in the history 
* Fix the reloc type typo

* do not track tls_index reference

* Update guid so the new collection contains correct reloc data

* fix the code for Add

* remove old code and add comment

* jit format

* Add Arm64 target in reproNative.vcxproj

* jit format

* review feedback

* jit format
  • Loading branch information
@kunalspathak
kunalspathak committed Jul 20, 2024
1 parent 2f0920c commit 922c2d8
Show file tree
Hide file tree
Showing 11 changed files with 183 additions and 28 deletions.
10 changes: 5 additions & 5 deletions src/coreclr/inc/jiteeversionguid.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID;
#define GUID_DEFINED
#endif // !GUID_DEFINED

constexpr GUID JITEEVersionIdentifier = { /* 273ba350-32bf-4714-beb0-7fa46c11364d */
0x273ba350,
0x32bf,
0x4714,
{0xbe, 0xb0, 0x7f, 0xa4, 0x6c, 0x11, 0x36, 0x4d}
constexpr GUID JITEEVersionIdentifier = { /* 488a17ce-26c9-4ad0-a7b7-79bf320ea4d1 */
0x488a17ce,
0x26c9,
0x4ad0,
{0xa7, 0xb7, 0x79, 0xbf, 0x32, 0x0e, 0xa4, 0xd1}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
20 changes: 13 additions & 7 deletions src/coreclr/jit/codegenarm64.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2236,18 +2236,12 @@ void CodeGen::instGen_Set_Reg_To_Imm(emitAttr size,
// reg cannot be a FP register
assert(!genIsValidFloatReg(reg));

emitAttr origAttr = size;
if (!compiler->opts.compReloc)
{
size = EA_SIZE(size); // Strip any Reloc flags from size if we aren't doing relocs
}

if (compiler->IsTargetAbi(CORINFO_NATIVEAOT_ABI) && EA_IS_CNS_SEC_RELOC(origAttr))
{
// This emits pair of `add` instructions for TLS reloc
GetEmitter()->emitIns_Add_Add_Tls_Reloc(size, reg, imm DEBUGARG(gtFlags));
}
else if (EA_IS_RELOC(size))
if (EA_IS_RELOC(size))
{
// This emits a pair of adrp/add (two instructions) with fix-ups.
GetEmitter()->emitIns_R_AI(INS_adrp, size, reg, imm DEBUGARG(targetHandle) DEBUGARG(gtFlags));
Expand Down Expand Up @@ -2764,6 +2758,18 @@ void CodeGen::genCodeForBinary(GenTreeOp* tree)
genProduceReg(tree);
return;
}
else if (compiler->IsTargetAbi(CORINFO_NATIVEAOT_ABI) && TargetOS::IsWindows &&
op2->IsIconHandle(GTF_ICON_SECREL_OFFSET))
{
// This emits pair of `add` instructions for TLS reloc on windows/arm64/nativeaot
assert(op2->AsIntCon()->ImmedValNeedsReloc(compiler));

emitAttr attr = emitActualTypeSize(targetType);
attr = EA_SET_FLG(attr, EA_CNS_RELOC_FLG | EA_CNS_SEC_RELOC);

emit->emitIns_Add_Add_Tls_Reloc(attr, targetReg, op1->GetRegNum(), op2->AsIntCon()->IconValue());
return;
}

instruction ins = genGetInsForOper(tree->OperGet(), targetType);

Expand Down
30 changes: 25 additions & 5 deletions src/coreclr/jit/emitarm64.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3756,6 +3756,7 @@ void emitter::emitIns_R(instruction ins, emitAttr attr, regNumber reg, insOpts o
// gtFlags - DEBUG only gtFlags.
//
void emitter::emitIns_Add_Add_Tls_Reloc(emitAttr attr,
regNumber targetReg,
regNumber reg,
ssize_t imm DEBUGARG(GenTreeFlags gtFlags /* = GTF_EMPTY */))
{
Expand All @@ -3777,7 +3778,7 @@ void emitter::emitIns_Add_Add_Tls_Reloc(emitAttr attr,
id->idInsOpt(INS_OPTS_LSL12);
id->idAddr()->iiaAddr = (BYTE*)imm;

id->idReg1(reg);
id->idReg1(targetReg);
id->idReg2(reg);

// Since this is relocation, set to 8 byte size.
Expand All @@ -3804,7 +3805,7 @@ void emitter::emitIns_Add_Add_Tls_Reloc(emitAttr attr,
id->idInsFmt(fmt);
id->idAddr()->iiaAddr = (BYTE*)imm;

id->idReg1(reg);
id->idReg1(targetReg);
id->idReg2(reg);

// Since this is relocation, set to 8 byte size.
Expand Down Expand Up @@ -11349,7 +11350,7 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
else
{
// This is second "add" of "add/add" pair
emitRecordRelocation(odst, id->idAddr()->iiaAddr, IMAGE_REL_ARM64_SECREL_LOW12L);
emitRecordRelocation(odst, id->idAddr()->iiaAddr, IMAGE_REL_ARM64_SECREL_LOW12A);
}
}
else
Expand Down Expand Up @@ -13539,13 +13540,32 @@ void emitter::emitDispInsHelp(
if (id->idIsReloc())
{
assert(ins == INS_add);
printf("[LOW RELOC ");

if (emitComp->IsTargetAbi(CORINFO_NATIVEAOT_ABI) && TargetOS::IsWindows && id->idIsTlsGD())
{
printf("[HIGH RELOC ");
}
else
{
printf("[LOW RELOC ");
}

emitDispImm((ssize_t)id->idAddr()->iiaAddr, false);
printf("]");
}
else
{
emitDispImmOptsLSL(emitGetInsSC(id), insOptsLSL12(id->idInsOpt()), 12);
if (emitComp->IsTargetAbi(CORINFO_NATIVEAOT_ABI) && TargetOS::IsWindows && id->idIsTlsGD())
{
assert(ins == INS_add);
printf("[LOW RELOC ");
emitDispImm((ssize_t)id->idAddr()->iiaAddr, false);
printf("]");
}
else
{
emitDispImmOptsLSL(emitGetInsSC(id), insOptsLSL12(id->idInsOpt()), 12);
}
}
break;

Expand Down
5 changes: 4 additions & 1 deletion src/coreclr/jit/emitarm64.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1455,7 +1455,10 @@ void emitIns_R_I(instruction ins,
insOpts opt = INS_OPTS_NONE,
insScalableOpts sopt = INS_SCALABLE_OPTS_NONE DEBUGARG(size_t targetHandle = 0)
DEBUGARG(GenTreeFlags gtFlags = GTF_EMPTY));
void emitIns_Add_Add_Tls_Reloc(emitAttr attr, regNumber reg, ssize_t imm DEBUGARG(GenTreeFlags gtFlags = GTF_EMPTY));
void emitIns_Add_Add_Tls_Reloc(emitAttr attr,
regNumber targetReg,
regNumber reg,
ssize_t imm DEBUGARG(GenTreeFlags gtFlags = GTF_EMPTY));

void emitInsSve_R_I(instruction ins,
emitAttr attr,
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/helperexpansion.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ bool Compiler::fgExpandThreadLocalAccessForCallNativeAOT(BasicBlock** pBlock, St

CORINFO_CONST_LOOKUP tlsIndexObject = threadStaticInfo.tlsIndexObject;

GenTree* dllRef = gtNewIconHandleNode((size_t)tlsIndexObject.handle, GTF_ICON_OBJ_HDL);
GenTree* dllRef = gtNewIconHandleNode((size_t)tlsIndexObject.handle, GTF_ICON_CONST_PTR);
dllRef = gtNewIndir(TYP_INT, dllRef, GTF_IND_NONFAULTING | GTF_IND_INVARIANT);
dllRef = gtNewCastNode(TYP_I_IMPL, dllRef, true, TYP_I_IMPL);
dllRef = gtNewOperNode(GT_LSH, TYP_I_IMPL, dllRef, gtNewIconNode(3, TYP_I_IMPL));
Expand Down
15 changes: 14 additions & 1 deletion src/coreclr/jit/lowerarmarch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,20 @@ bool Lowering::IsContainableImmed(GenTree* parentNode, GenTree* childNode) const
if (!childNode->IsCnsIntOrI())
return false;
if (childNode->AsIntCon()->ImmedValNeedsReloc(comp))
return false;
{
if (comp->IsTargetAbi(CORINFO_NATIVEAOT_ABI) && TargetOS::IsWindows &&
childNode->IsIconHandle(GTF_ICON_SECREL_OFFSET))
{
// for windows/arm64, the immediate constant should be contained because it gets
// generated as part of ADD instruction that consumes this constant. See
// emitIns_Add_Add_Tls_Reloc().
return true;
}
else
{
return false;
}
}

// TODO-CrossBitness: we wouldn't need the cast below if GenTreeIntCon::gtIconVal had target_ssize_t type.
target_ssize_t immVal = (target_ssize_t)childNode->AsIntCon()->gtIconVal;
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/tools/Common/Compiler/DependencyAnalysis/Relocation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public enum RelocType

// Windows arm64 TLS access
IMAGE_REL_ARM64_TLS_SECREL_HIGH12A = 0x10F, // ADD high 12-bit offset for tls
IMAGE_REL_ARM64_TLS_SECREL_LOW12L = 0x110, // ADD low 12-bit offset for tls
IMAGE_REL_ARM64_TLS_SECREL_LOW12A = 0x110, // ADD low 12-bit offset for tls

//
// Relocations for R2R image production
Expand Down Expand Up @@ -536,7 +536,7 @@ public static unsafe void WriteValue(RelocType relocType, void* location, long v
break;
case RelocType.IMAGE_REL_BASED_ARM64_PAGEOFFSET_12A:
case RelocType.IMAGE_REL_AARCH64_TLSDESC_ADD_LO12:
case RelocType.IMAGE_REL_ARM64_TLS_SECREL_LOW12L:
case RelocType.IMAGE_REL_ARM64_TLS_SECREL_LOW12A:
PutArm64Rel12((uint*)location, (int)value);
break;
case RelocType.IMAGE_REL_BASED_LOONGARCH64_PC:
Expand Down Expand Up @@ -593,7 +593,7 @@ public static unsafe long ReadValue(RelocType relocType, void* location)
return GetArm64Rel21((uint*)location);
case RelocType.IMAGE_REL_BASED_ARM64_PAGEOFFSET_12A:
case RelocType.IMAGE_REL_ARM64_TLS_SECREL_HIGH12A:
case RelocType.IMAGE_REL_ARM64_TLS_SECREL_LOW12L:
case RelocType.IMAGE_REL_ARM64_TLS_SECREL_LOW12A:
return GetArm64Rel12((uint*)location);
case RelocType.IMAGE_REL_AARCH64_TLSDESC_LD64_LO12:
case RelocType.IMAGE_REL_AARCH64_TLSDESC_ADD_LO12:
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3920,8 +3920,8 @@ private static RelocType GetRelocType(TargetArchitecture targetArchitecture, ush
const ushort IMAGE_REL_ARM64_BRANCH26 = 3;
const ushort IMAGE_REL_ARM64_PAGEBASE_REL21 = 4;
const ushort IMAGE_REL_ARM64_PAGEOFFSET_12A = 6;
const ushort IMAGE_REL_ARM64_SECREL_LOW12A = 9;
const ushort IMAGE_REL_ARM64_SECREL_HIGH12A = 0xA;
const ushort IMAGE_REL_ARM64_SECREL_LOW12L = 0xB;
const ushort IMAGE_REL_ARM64_TLSDESC_ADR_PAGE21 = 0x107;
const ushort IMAGE_REL_ARM64_TLSDESC_LD64_LO12 = 0x108;
const ushort IMAGE_REL_ARM64_TLSDESC_ADD_LO12 = 0x109;
Expand All @@ -3946,8 +3946,8 @@ private static RelocType GetRelocType(TargetArchitecture targetArchitecture, ush
return RelocType.IMAGE_REL_AARCH64_TLSDESC_CALL;
case IMAGE_REL_ARM64_SECREL_HIGH12A:
return RelocType.IMAGE_REL_ARM64_TLS_SECREL_HIGH12A;
case IMAGE_REL_ARM64_SECREL_LOW12L:
return RelocType.IMAGE_REL_ARM64_TLS_SECREL_LOW12L;
case IMAGE_REL_ARM64_SECREL_LOW12A:
return RelocType.IMAGE_REL_ARM64_TLS_SECREL_LOW12A;
default:
Debug.Fail("Invalid RelocType: " + fRelocType);
return 0;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/CoffObjectWriter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ private protected override void EmitRelocations(int sectionIndex, List<SymbolicR
IMAGE_REL_BASED_ARM64_PAGEBASE_REL21 => IMAGE_REL_ARM64_PAGEBASE_REL21,
IMAGE_REL_BASED_ARM64_PAGEOFFSET_12A => IMAGE_REL_ARM64_PAGEOFFSET_12A,
IMAGE_REL_ARM64_TLS_SECREL_HIGH12A => IMAGE_REL_ARM64_SECREL_HIGH12A,
IMAGE_REL_ARM64_TLS_SECREL_LOW12L => IMAGE_REL_ARM64_SECREL_LOW12L,
IMAGE_REL_ARM64_TLS_SECREL_LOW12A => IMAGE_REL_ARM64_SECREL_LOW12A,
IMAGE_REL_SECREL => IMAGE_REL_ARM64_SECREL,
IMAGE_REL_SECTION => IMAGE_REL_ARM64_SECTION,
_ => throw new NotSupportedException($"Unsupported relocation: {relocation.Type}")
Expand Down
Loading

0 comments on commit 922c2d8

Please sign in to comment.