Skip to content
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

NativeAOT/win/arm64: Fix the reloc type typo #104516

Merged
merged 13 commits into from
Jul 20, 2024
10 changes: 5 additions & 5 deletions src/coreclr/inc/jiteeversionguid.h
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 = { /* 64fe30fb-0a27-4b1d-b89e-306e552ac848 */
0x64fe30fb,
0x0a27,
0x4b1d,
{0xb8, 0x9e, 0x30, 0x6e, 0x55, 0x2a, 0xc8, 0x48}
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
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))
Comment on lines +2761 to +2762
Copy link
Member

Choose a reason for hiding this comment

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

So this constant isn't considered a reloc?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is and hence we add EA_CNS_RELOC_FLG | EA_CNS_SEC_RELOC flags below.

{
// 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
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
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
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
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -3915,8 +3915,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 @@ -3941,8 +3941,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
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
Loading