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] Inline TLS access for windows/x64 #89472

Merged
merged 64 commits into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
f1c5228
wip
kunalspathak Jul 7, 2023
91ee21f
working model
kunalspathak Jul 10, 2023
fe242f1
wip
kunalspathak Jul 11, 2023
fcf00d2
wip
kunalspathak Jul 21, 2023
f736b2c
working
kunalspathak Jul 25, 2023
6cd92f3
Add helper for tlsIndex
kunalspathak Jul 25, 2023
87c3f25
add methods in superpmi
kunalspathak Jul 25, 2023
fb364a0
revert some local changes
kunalspathak Jul 25, 2023
e9610c6
misc fixes
kunalspathak Jul 25, 2023
86d01a7
Stop emitting TLS access code for windows/x64
kunalspathak Jul 25, 2023
09eaeb4
fix linux build errors
kunalspathak Jul 28, 2023
337c2f4
Do not throw not implemented for windows/x64
kunalspathak Jul 28, 2023
1209465
fix the problem where ThreadStaticBase helper was still getting invoked
kunalspathak Jul 28, 2023
85e1db6
Revert certain changes from JIT method
kunalspathak Jul 28, 2023
a3d3070
Introduce getThreadLocalStaticInfo_ReadyToRun()
kunalspathak Jul 29, 2023
800005d
Consume getThreadLocalStaticInfo_ReadyToRun()
kunalspathak Jul 29, 2023
4766211
Remove getTlsRootInfo() and other methods
kunalspathak Jul 29, 2023
b9adff7
Revert unneeded changes
kunalspathak Jul 29, 2023
9ca78ae
missing gtInitCldHnd initialization
kunalspathak Jul 30, 2023
b142d28
save target address
kunalspathak Jul 30, 2023
79de728
jit format
kunalspathak Jul 30, 2023
25482b9
Merge remote-tracking branch 'origin/main' into tls_nativeaot_winx64
kunalspathak Nov 20, 2023
1fe8103
run thunkgenerator
kunalspathak Nov 20, 2023
7c03b66
resolve merge conflicts
kunalspathak Dec 1, 2023
07c839b
fix issues so the TLS is inlined
kunalspathak Dec 1, 2023
65ceeaa
Rename data structures from *_ReadyToRun to *_NativeAOT
kunalspathak Dec 7, 2023
8af0e6e
Merge remote-tracking branch 'origin/main' into working_tls_nativeaot…
kunalspathak Dec 7, 2023
fc8e2b8
jit format
kunalspathak Dec 7, 2023
d7e7f9d
fix some unit test
kunalspathak Dec 8, 2023
b70ceeb
fix a bug
kunalspathak Dec 8, 2023
f6552f2
fix the weird jump problem
kunalspathak Dec 9, 2023
57893d8
use enclosing type cls handle for VN of static gc/non-gc helper
kunalspathak Dec 10, 2023
1537ecd
fix a bug of resetting the flag
kunalspathak Dec 11, 2023
ab95bb9
useEnclosingTypeOnly from runtime to determine if VN should optimize it
kunalspathak Dec 13, 2023
c9040a2
do not use vnf, but only use useEnclosingTypeAsArg0
kunalspathak Dec 14, 2023
5f76a6c
Use GT_COMMA to add GCStaticBase call next to TLS call
kunalspathak Dec 14, 2023
3d16881
Merge remote-tracking branch 'kp/vn_staticbase' into working_tls_nati…
kunalspathak Dec 14, 2023
daca626
optimize the cctor call
kunalspathak Dec 14, 2023
c7d75ef
Remove lazy ctor generation from tls
kunalspathak Dec 15, 2023
5895ab3
Update jitinterface to not fetch data for lazy ctor
kunalspathak Dec 15, 2023
d53f324
Merge remote-tracking branch 'origin/main' into working_tls_nativeaot…
kunalspathak Dec 15, 2023
571b6df
fix errors after merge
kunalspathak Dec 15, 2023
da95659
fix test build errors
kunalspathak Dec 16, 2023
8679e4b
fix bug in CSE
kunalspathak Dec 16, 2023
0ce7618
Use CORINFO_FLG_FIELD_INITCLASS instead of separate flag
kunalspathak Dec 18, 2023
b38a17c
Use the INITCLASS flag
kunalspathak Dec 18, 2023
1c1092d
Remove useEnclosingTypeOnly
kunalspathak Dec 19, 2023
119e2ae
Add NoCtor
kunalspathak Dec 19, 2023
9cfea39
Use CORINFO_HELP_READYTORUN_THREADSTATIC_BASE_NOCTOR
kunalspathak Dec 19, 2023
2cc3ca1
Minor cleanup
kunalspathak Dec 19, 2023
8389a88
Merge remote-tracking branch 'origin/main' into tls_nativeaot_winx64
kunalspathak Jan 3, 2024
59085d2
Renegenrate thunk
kunalspathak Jan 3, 2024
fa1059f
Add the SetFalseTarget
kunalspathak Jan 4, 2024
63c9fd6
Merge remote-tracking branch 'origin/main' into tls_nativeaot_winx64
kunalspathak Jan 10, 2024
0069167
fix merge conflict resolution
kunalspathak Jan 10, 2024
38bca3b
better handling of GTF_ICON_SECREL_OFFSET
kunalspathak Jan 10, 2024
a5fcc59
review feedback
kunalspathak Jan 10, 2024
ceda478
Disable optimization for minopts
kunalspathak Jan 11, 2024
aba00e2
Add comments around iiaSecRel
kunalspathak Jan 11, 2024
24f88a0
jit format
kunalspathak Jan 11, 2024
65f3900
create emitNewInstrCns()
kunalspathak Jan 11, 2024
5bc0a1a
Expand TLS even if optimization is disabled
kunalspathak Jan 12, 2024
959869a
Track t_inlinedThreadStaticBase
kunalspathak Jan 12, 2024
bff4728
jit format
kunalspathak Jan 13, 2024
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
7 changes: 7 additions & 0 deletions src/coreclr/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -1684,6 +1684,7 @@ enum CORINFO_FIELD_ACCESSOR
CORINFO_FIELD_STATIC_ADDR_HELPER, // static field accessed using address-of helper (argument is FieldDesc *)
CORINFO_FIELD_STATIC_TLS, // unmanaged TLS access
CORINFO_FIELD_STATIC_TLS_MANAGED, // managed TLS access
CORINFO_FIELD_STATIC_TLS_MANAGED_LAZY, // managed TLS access lazy ctor initialization
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better for this to be a flag instead of a new type of field accessor?

There is CORINFO_FLG_FIELD_INITCLASS that seems to be what we need.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is CORINFO_FLG_FIELD_INITCLASS that seems to be what we need.

This flag solved lot of the problems.

  • No need for CORINFO_FIELD_STATIC_TLS_MANAGED_LAZY
  • No need to add the extra GCStaticBase helper call during importing
  • No need of tracking enclosingType and using it in VN
  • The gtEntryPoint gets setup to correct symbol
  • Eliminates the extra thread static helper remaining discussed in [NativeAOT] Inline TLS access for windows/x64 #89472 (comment)

CORINFO_FIELD_STATIC_READYTORUN_HELPER, // static field access using a runtime lookup helper
CORINFO_FIELD_STATIC_RELOCATABLE, // static field access using relocation (used in AOT)
CORINFO_FIELD_INTRINSIC_ZERO, // intrinsic zero (IntPtr.Zero, UIntPtr.Zero)
Expand Down Expand Up @@ -2789,6 +2790,11 @@ class ICorStaticInfo
bool isGCType
) = 0;

virtual void getTlsRootInfo(CORINFO_CONST_LOOKUP* addr) = 0;
virtual void getTlsIndexInfo(CORINFO_CONST_LOOKUP* addr) = 0;
virtual void getThreadStaticBaseSlowInfo(CORINFO_CONST_LOOKUP* addr) = 0;
virtual int getEnsureClassCtorRunAndReturnThreadStaticBaseHelper(CORINFO_CLASS_HANDLE cls, CORINFO_CONST_LOOKUP* addr, CORINFO_CONST_LOOKUP* targetSymbol) = 0;

// Returns true iff "fldHnd" represents a static field.
virtual bool isFieldStatic(CORINFO_FIELD_HANDLE fldHnd) = 0;

Expand Down Expand Up @@ -3323,6 +3329,7 @@ class ICorDynamicInfo : public ICorStaticInfo
// It would be nicer to use existing IMAGE_REL_XXX constants instead of defining our own here...
#define IMAGE_REL_BASED_REL32 0x10
#define IMAGE_REL_BASED_THUMB_BRANCH24 0x13
#define IMAGE_REL_SECREL 0x104

// The identifier for ARM32-specific PC-relative address
// computation corresponds to the following instruction
Expand Down
14 changes: 14 additions & 0 deletions src/coreclr/inc/icorjitinfoimpl_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,20 @@ void getThreadLocalStaticBlocksInfo(
CORINFO_THREAD_STATIC_BLOCKS_INFO* pInfo,
bool isGCType) override;

void getTlsRootInfo(
CORINFO_CONST_LOOKUP* addr) override;

void getTlsIndexInfo(
CORINFO_CONST_LOOKUP* addr) override;

void getThreadStaticBaseSlowInfo(
CORINFO_CONST_LOOKUP* addr) override;

int getEnsureClassCtorRunAndReturnThreadStaticBaseHelper(
CORINFO_CLASS_HANDLE cls,
CORINFO_CONST_LOOKUP* addr,
CORINFO_CONST_LOOKUP* targetSymbol) override;

bool isFieldStatic(
CORINFO_FIELD_HANDLE fldHnd) override;

Expand Down
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 = { /* cef79bc8-29bf-4f7b-9d05-9fc06832098c */
0xcef79bc8,
0x29bf,
0x4f7b,
{0x9d, 0x05, 0x9f, 0xc0, 0x68, 0x32, 0x09, 0x8c}
constexpr GUID JITEEVersionIdentifier = { /* ae2c23aa-c919-44b9-a1b8-bed99aad9d20 */
0xae2c23aa,
0xc919,
0x44b9,
{0xa1, 0xb8, 0xbe, 0xd9, 0x9a, 0xad, 0x9d, 0x20}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/jit/ICorJitInfo_names_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ DEF_CLR_API(getFieldOffset)
DEF_CLR_API(getFieldInfo)
DEF_CLR_API(getThreadLocalFieldInfo)
DEF_CLR_API(getThreadLocalStaticBlocksInfo)
DEF_CLR_API(getTlsRootInfo)
DEF_CLR_API(getTlsIndexInfo)
DEF_CLR_API(getThreadStaticBaseSlowInfo)
DEF_CLR_API(getEnsureClassCtorRunAndReturnThreadStaticBaseHelper)
DEF_CLR_API(isFieldStatic)
DEF_CLR_API(getArrayOrStringLength)
DEF_CLR_API(getBoundaries)
Expand Down
35 changes: 35 additions & 0 deletions src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -932,6 +932,41 @@ void WrapICorJitInfo::getThreadLocalStaticBlocksInfo(
API_LEAVE(getThreadLocalStaticBlocksInfo);
}

void WrapICorJitInfo::getTlsRootInfo(
CORINFO_CONST_LOOKUP* addr)
{
API_ENTER(getTlsRootInfo);
wrapHnd->getTlsRootInfo(addr);
API_LEAVE(getTlsRootInfo);
}

void WrapICorJitInfo::getTlsIndexInfo(
CORINFO_CONST_LOOKUP* addr)
{
API_ENTER(getTlsIndexInfo);
wrapHnd->getTlsIndexInfo(addr);
API_LEAVE(getTlsIndexInfo);
}

void WrapICorJitInfo::getThreadStaticBaseSlowInfo(
CORINFO_CONST_LOOKUP* addr)
{
API_ENTER(getThreadStaticBaseSlowInfo);
wrapHnd->getThreadStaticBaseSlowInfo(addr);
API_LEAVE(getThreadStaticBaseSlowInfo);
}

int WrapICorJitInfo::getEnsureClassCtorRunAndReturnThreadStaticBaseHelper(
CORINFO_CLASS_HANDLE cls,
CORINFO_CONST_LOOKUP* addr,
CORINFO_CONST_LOOKUP* targetSymbol)
{
API_ENTER(getEnsureClassCtorRunAndReturnThreadStaticBaseHelper);
int temp = wrapHnd->getEnsureClassCtorRunAndReturnThreadStaticBaseHelper(cls, addr, targetSymbol);
API_LEAVE(getEnsureClassCtorRunAndReturnThreadStaticBaseHelper);
return temp;
}

bool WrapICorJitInfo::isFieldStatic(
CORINFO_FIELD_HANDLE fldHnd)
{
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,11 @@ void CodeGen::genSetRegToConst(regNumber targetReg, var_types targetType, GenTre
attr = EA_SET_FLG(attr, EA_BYREF_FLG);
}

if (con->IsIconHandle(GTF_ICON_SECREL_OFFSET))
{
attr = EA_SET_FLG(attr, EA_CNS_SEC_RELOC);
}

instGen_Set_Reg_To_Imm(attr, targetReg, cnsVal,
INS_FLAGS_DONT_CARE DEBUGARG(con->gtTargetHandle) DEBUGARG(con->gtFlags));
regSet.verifyRegUsed(targetReg);
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -5355,6 +5355,7 @@ class Compiler

PhaseStatus fgExpandThreadLocalAccess();
bool fgExpandThreadLocalAccessForCall(BasicBlock** pBlock, Statement* stmt, GenTreeCall* call);
bool fgExpandThreadLocalAccessForCallReadyToRun(BasicBlock** pBlock, Statement* stmt, GenTreeCall* call);

PhaseStatus fgExpandStaticInit();
bool fgExpandStaticInitForCall(BasicBlock** pBlock, Statement* stmt, GenTreeCall* call);
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/emit.h
Original file line number Diff line number Diff line change
Expand Up @@ -1017,7 +1017,7 @@ class emitter
return iiaJmpOffset;
}
#endif // defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64)

bool isSecRel;
kunalspathak marked this conversation as resolved.
Show resolved Hide resolved
} _idAddrUnion;

/* Trivial wrappers to return properly typed enums */
Expand Down
12 changes: 11 additions & 1 deletion src/coreclr/jit/emitxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5818,6 +5818,11 @@ void emitter::emitIns_R_I(instruction ins,
id->idDebugOnlyInfo()->idMemCookie = targetHandle;
#endif

if (emitComp->opts.IsReadyToRun() && EA_IS_CNS_SEC_RELOC(attr))
{
id->idAddr()->isSecRel = true;
}

kunalspathak marked this conversation as resolved.
Show resolved Hide resolved
if (isSimdInsAndValInByte)
{
bool includeRexPrefixSize = true;
Expand Down Expand Up @@ -15147,7 +15152,12 @@ BYTE* emitter::emitOutputRI(BYTE* dst, instrDesc* id)

if (id->idIsCnsReloc())
{
emitRecordRelocation((void*)(dst - (unsigned)EA_SIZE(size)), (void*)(size_t)val, IMAGE_REL_BASED_MOFFSET);
uint16_t relocationType = IMAGE_REL_BASED_MOFFSET;
if (emitComp->opts.IsReadyToRun() && id->idAddr()->isSecRel)
{
relocationType = IMAGE_REL_SECREL;
}
emitRecordRelocation((void*)(dst - (unsigned)EA_SIZE(size)), (void*)(size_t)val, relocationType);
}

goto DONE;
Expand Down
17 changes: 17 additions & 0 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,7 @@ enum GenTreeFlags : unsigned int
GTF_ICON_STATIC_BOX_PTR = 0x10000000, // GT_CNS_INT -- constant is an address of the box for a STATIC_IN_HEAP field
GTF_ICON_FIELD_SEQ = 0x11000000, // <--------> -- constant is a FieldSeq* (used only as VNHandle)
GTF_ICON_STATIC_ADDR_PTR = 0x13000000, // GT_CNS_INT -- constant is a pointer to a static base address
GTF_ICON_SECREL_OFFSET = 0x14000000, // GT_CNS_INT -- constant is an offset in a certain section.

// GTF_ICON_REUSE_REG_VAL = 0x00800000 // GT_CNS_INT -- GTF_REUSE_REG_VAL, defined above
GTF_ICON_SIMD_COUNT = 0x00200000, // GT_CNS_INT -- constant is Vector<T>.Count
Expand Down Expand Up @@ -4059,6 +4060,7 @@ enum GenTreeCallFlags : unsigned int
GTF_CALL_M_HAS_LATE_DEVIRT_INFO = 0x10000000, // this call has late devirtualzation info
GTF_CALL_M_LDVIRTFTN_INTERFACE = 0x20000000, // ldvirtftn on an interface type
GTF_CALL_M_EXP_TLS_ACCESS = 0x40000000, // this call is a helper for access TLS marked field
GTF_CALL_M_EXP_TLS_ACCESS_LAZY = 0x80000000, // this call is a helper for access TLS marked field and has lazy ctor
};

inline constexpr GenTreeCallFlags operator ~(GenTreeCallFlags a)
Expand Down Expand Up @@ -5409,6 +5411,21 @@ struct GenTreeCall final : public GenTree
return (gtCallMoreFlags & GTF_CALL_M_EXP_TLS_ACCESS) != 0;
}

void SetExpTLSFieldAccessLazyCtor()
{
gtCallMoreFlags |= GTF_CALL_M_EXP_TLS_ACCESS_LAZY;
}

void ClearExpTLSFieldAccessLazyCtor()
{
gtCallMoreFlags &= ~GTF_CALL_M_EXP_TLS_ACCESS_LAZY;
}

bool IsExpTLSFieldAccessLazyCtor() const
{
return (gtCallMoreFlags & GTF_CALL_M_EXP_TLS_ACCESS_LAZY) != 0;
}

void SetExpandedEarly()
{
gtCallMoreFlags |= GTF_CALL_M_EXPANDED_EARLY;
Expand Down
Loading
Loading