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

Build infra changes to enable building a more universal cross target jit builds #41126

Merged
merged 23 commits into from
Sep 9, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
906d468
Code changes needed to compile targetting all supported os/arch from …
davidwrighton Aug 17, 2020
83ed473
Fix unix to windows cross target build issues
davidwrighton Aug 20, 2020
482e8c3
Build rules to build all possible altjits
davidwrighton Aug 18, 2020
824419d
New build subset rules
davidwrighton Aug 19, 2020
a56cc15
Adjust naming to clrjit_targetos_targetarch_hostarch
davidwrighton Aug 20, 2020
3d359df
Fix subsets build so that clr subset compiles properly
davidwrighton Aug 20, 2020
d471312
Swap crossgen2 to use new clrjit dlls
davidwrighton Aug 21, 2020
1afd7e1
Use target defintions and properties to simplify logic
davidwrighton Aug 21, 2020
72dd26d
Stop using subdirectories for cross targeting jits and such
davidwrighton Aug 21, 2020
ef86678
Consolidate all standalone jit building to use the function instead o…
davidwrighton Aug 22, 2020
7d3c823
Fix build break, and apply formatting patch
davidwrighton Aug 24, 2020
db74ff8
Address code review feedback, and fix pgo build and armel jit build
davidwrighton Aug 24, 2020
d7a5a91
Fix Windows Arm build, and don't compile clrjit as altjit, ever
davidwrighton Aug 25, 2020
8b184d7
Fix crossgen2 packaging on Unix
davidwrighton Aug 25, 2020
fa08b56
Code review feedback
davidwrighton Aug 26, 2020
0f420f6
Make jitinterface be host specific, and fix build of windows crosstar…
davidwrighton Sep 2, 2020
bbeceaf
Fix issue blocking compilation of X86 targetted code
davidwrighton Sep 2, 2020
b0703f7
Improve R2R dump
davidwrighton Sep 2, 2020
f5e1064
Fix difference in x86 r2r image generation between x64 and x86 hosted
davidwrighton Sep 2, 2020
c2a2789
Address code review feedback
davidwrighton Sep 3, 2020
42b44da
Fix issues found in CI
davidwrighton Sep 4, 2020
92fe566
Fix formatting
davidwrighton Sep 4, 2020
8da95fb
add comment as requested
davidwrighton Sep 4, 2020
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
2 changes: 1 addition & 1 deletion eng/Signing.props
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
<ItemsToSign Include="$(CoreCLRCrossgen2Dir)ILCompiler.DependencyAnalysisFramework.dll" />
<ItemsToSign Include="$(CoreCLRCrossgen2Dir)ILCompiler.ReadyToRun.dll" />
<ItemsToSign Include="$(CoreCLRCrossgen2Dir)ILCompiler.TypeSystem.ReadyToRun.dll" />
<ItemsToSign Include="$(CoreCLRCrossgen2Dir)jitinterface.dll" />
<ItemsToSign Include="$(CoreCLRCrossgen2Dir)jitinterface_$(TargetArchitecture).dll" />

<ItemsToSign Include="$(CoreCLRCrossgen2Dir)clrjit_win_x86_$(TargetArchitecture).dll" />
<ItemsToSign Include="$(CoreCLRCrossgen2Dir)clrjit_win_arm_$(TargetArchitecture).dll" />
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/crosscomponents.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ endif(CLR_CMAKE_HOST_ARCH_AMD64 AND (CLR_CMAKE_TARGET_ARCH_ARM OR CLR_CMAKE_TARG
if (CLR_CMAKE_HOST_OS STREQUAL CLR_CMAKE_TARGET_OS)
set (CLR_CROSS_COMPONENTS_LIST
clrjit
jitinterface
jitinterface_${ARCH_HOST_NAME}
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need multiple jitinterface-wrapper libraries?

(Since crossgen2 basically ships selfcontained with the SDK, wouldn't the bitness of jitinterface.dll always match the bitness of coreclr.dll that the compiler is running with?)

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't need them in the shipping scenarios, but it enables some mix and matching of bits in some test infra work I'm lookin at. In particular, it makes it fairly simple to have a crossgen2 that is easy to use in crosstargeting scenarios in the core_root directory, especially on Windows. (Since the default available dotnet in our Windows build environment is 64bit only, this small tweak makes it easy to have a crossgen2 that will just work for developers)

)

if(CLR_CMAKE_HOST_LINUX OR NOT FEATURE_CROSSBITNESS)
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/src/jit/codegen.h
Original file line number Diff line number Diff line change
Expand Up @@ -1393,8 +1393,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
emitAttr size,
insFlags flags = INS_FLAGS_DONT_CARE);

void inst_IV(instruction ins, int val);
void inst_IV_handle(instruction ins, int val);
void inst_IV(instruction ins, cnsval_ssize_t val);
void inst_IV_handle(instruction ins, cnsval_ssize_t val);

void inst_RV_IV(
instruction ins, regNumber reg, target_ssize_t val, emitAttr size, insFlags flags = INS_FLAGS_DONT_CARE);
Expand Down
18 changes: 8 additions & 10 deletions src/coreclr/src/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7519,11 +7519,11 @@ void CodeGen::genPutArgStkFieldList(GenTreePutArgStk* putArgStk)
case GT_CNS_INT:
if (fieldNode->IsIconHandle())
{
inst_IV_handle(INS_push, (target_ssize_t)fieldNode->AsIntCon()->gtIconVal);
inst_IV_handle(INS_push, fieldNode->AsIntCon()->gtIconVal);
}
else
{
inst_IV(INS_push, (target_ssize_t)fieldNode->AsIntCon()->gtIconVal);
inst_IV(INS_push, fieldNode->AsIntCon()->gtIconVal);
}
break;
default:
Expand Down Expand Up @@ -7618,11 +7618,11 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* putArgStk)
{
if (data->IsIconHandle())
{
inst_IV_handle(INS_push, (target_ssize_t)data->AsIntCon()->gtIconVal);
inst_IV_handle(INS_push, data->AsIntCon()->gtIconVal);
}
else
{
inst_IV(INS_push, (target_ssize_t)data->AsIntCon()->gtIconVal);
inst_IV(INS_push, data->AsIntCon()->gtIconVal);
}
AddStackLevel(argSize);
}
Expand Down Expand Up @@ -8493,12 +8493,11 @@ void CodeGen::genProfilingEnterCallback(regNumber initReg, bool* pInitRegZeroed)
// Push the profilerHandle
if (compiler->compProfilerMethHndIndirected)
{
GetEmitter()->emitIns_AR_R(INS_push, EA_PTR_DSP_RELOC, REG_NA, REG_NA,
(target_ssize_t)(size_t)compiler->compProfilerMethHnd);
GetEmitter()->emitIns_AR_R(INS_push, EA_PTR_DSP_RELOC, REG_NA, REG_NA, (ssize_t)compiler->compProfilerMethHnd);
}
else
{
inst_IV(INS_push, (target_ssize_t)(size_t)compiler->compProfilerMethHnd);
inst_IV(INS_push, (size_t)compiler->compProfilerMethHnd);
}

//
Expand Down Expand Up @@ -8579,12 +8578,11 @@ void CodeGen::genProfilingLeaveCallback(unsigned helper)

if (compiler->compProfilerMethHndIndirected)
{
GetEmitter()->emitIns_AR_R(INS_push, EA_PTR_DSP_RELOC, REG_NA, REG_NA,
(target_ssize_t)(ssize_t)compiler->compProfilerMethHnd);
GetEmitter()->emitIns_AR_R(INS_push, EA_PTR_DSP_RELOC, REG_NA, REG_NA, (ssize_t)compiler->compProfilerMethHnd);
}
else
{
inst_IV(INS_push, (target_size_t)(size_t)compiler->compProfilerMethHnd);
inst_IV(INS_push, (size_t)compiler->compProfilerMethHnd);
}
genSinglePush();

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/src/jit/emit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6422,7 +6422,7 @@ unsigned char emitter::emitOutputLong(BYTE* dst, ssize_t val)

unsigned char emitter::emitOutputSizeT(BYTE* dst, ssize_t val)
{
#if !TARGET_64BIT
#if !defined(TARGET_64BIT)
MISALIGNED_WR_I4(dst, (int)val);
Copy link
Member

Choose a reason for hiding this comment

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

This seems wrong. It seems like we should rename emitOutputSizeT to emitOutputTargetSizeT and have it take a target_ssize_t.

Copy link
Member Author

Choose a reason for hiding this comment

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

Eh, I'd prefer naming changes be done by the jit team for the jit team in a separate pr. All of the emit functions take ssize_t, regardless of the size of the generated bits.

#else
MISALIGNED_WR_ST(dst, val);
Expand Down Expand Up @@ -7028,7 +7028,7 @@ void emitter::emitNxtIG(bool extend)
* emitGetInsSC: Get the instruction's constant value.
*/

target_ssize_t emitter::emitGetInsSC(instrDesc* id)
cnsval_ssize_t emitter::emitGetInsSC(instrDesc* id)
{
#ifdef TARGET_ARM // should it be TARGET_ARMARCH? Why do we need this? Note that on ARM64 we store scaled immediates
// for some formats
Expand Down
14 changes: 7 additions & 7 deletions src/coreclr/src/jit/emit.h
Original file line number Diff line number Diff line change
Expand Up @@ -1370,7 +1370,7 @@ class emitter

struct instrDescCns : instrDesc // large const
{
target_ssize_t idcCnsVal;
cnsval_ssize_t idcCnsVal;
};

struct instrDescDsp : instrDesc // large displacement
Expand Down Expand Up @@ -1466,7 +1466,7 @@ class emitter

#endif // TARGET_XARCH

target_ssize_t emitGetInsSC(instrDesc* id);
cnsval_ssize_t emitGetInsSC(instrDesc* id);
unsigned emitInsCount;

/************************************************************************/
Expand Down Expand Up @@ -1911,7 +1911,7 @@ class emitter
return (instrDescCns*)emitAllocAnyInstr(sizeof(instrDescCns), attr);
}

instrDescCns* emitAllocInstrCns(emitAttr attr, target_size_t cns)
instrDescCns* emitAllocInstrCns(emitAttr attr, cnsval_size_t cns)
{
instrDescCns* result = emitAllocInstrCns(attr);
result->idSetIsLargeCns();
Expand Down Expand Up @@ -1965,8 +1965,8 @@ class emitter

instrDesc* emitNewInstrSmall(emitAttr attr);
instrDesc* emitNewInstr(emitAttr attr = EA_4BYTE);
instrDesc* emitNewInstrSC(emitAttr attr, target_ssize_t cns);
instrDesc* emitNewInstrCns(emitAttr attr, target_ssize_t cns);
instrDesc* emitNewInstrSC(emitAttr attr, cnsval_ssize_t cns);
instrDesc* emitNewInstrCns(emitAttr attr, cnsval_ssize_t cns);
instrDesc* emitNewInstrDsp(emitAttr attr, target_ssize_t dsp);
instrDesc* emitNewInstrCnsDsp(emitAttr attr, target_ssize_t cns, int dsp);
#ifdef TARGET_ARM
Expand Down Expand Up @@ -2513,7 +2513,7 @@ inline emitter::instrDesc* emitter::emitNewInstrDsp(emitAttr attr, target_ssize_
* Note that this very similar to emitter::emitNewInstrSC(), except it never
* allocates a small descriptor.
*/
inline emitter::instrDesc* emitter::emitNewInstrCns(emitAttr attr, target_ssize_t cns)
inline emitter::instrDesc* emitter::emitNewInstrCns(emitAttr attr, cnsval_ssize_t cns)
{
if (instrDesc::fitsInSmallCns(cns))
{
Expand Down Expand Up @@ -2572,7 +2572,7 @@ inline size_t emitter::emitGetInstrDescSize(const instrDesc* id)
* emitNewInstrCns() always allocates at least sizeof(instrDesc)).
*/

inline emitter::instrDesc* emitter::emitNewInstrSC(emitAttr attr, target_ssize_t cns)
inline emitter::instrDesc* emitter::emitNewInstrSC(emitAttr attr, cnsval_ssize_t cns)
{
if (instrDesc::fitsInSmallCns(cns))
{
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/src/jit/emitxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3924,7 +3924,7 @@ void emitter::emitIns_R_I(instruction ins, emitAttr attr, regNumber reg, ssize_t
sz += emitGetRexPrefixSize(ins);
}

id = emitNewInstrSC(attr, (target_ssize_t)val);
id = emitNewInstrSC(attr, val);
id->idIns(ins);
id->idInsFmt(fmt);
id->idReg1(reg);
Expand All @@ -3945,7 +3945,7 @@ void emitter::emitIns_R_I(instruction ins, emitAttr attr, regNumber reg, ssize_t
* Add an instruction referencing an integer constant.
*/

void emitter::emitIns_I(instruction ins, emitAttr attr, int val)
void emitter::emitIns_I(instruction ins, emitAttr attr, cnsval_ssize_t val)
{
UNATIVE_OFFSET sz;
instrDesc* id;
Expand Down Expand Up @@ -5349,7 +5349,7 @@ void emitter::emitIns_R_AI(instruction ins, emitAttr attr, regNumber ireg, ssize
emitCurIGsize += sz;
}

void emitter::emitIns_AR_R(instruction ins, emitAttr attr, regNumber reg, regNumber base, int disp)
void emitter::emitIns_AR_R(instruction ins, emitAttr attr, regNumber reg, regNumber base, cnsval_ssize_t disp)
{
emitIns_ARX_R(ins, attr, reg, base, REG_NA, 1, disp);
}
Expand Down Expand Up @@ -5587,7 +5587,7 @@ void emitter::emitIns_R_ARX(
}

void emitter::emitIns_ARX_R(
instruction ins, emitAttr attr, regNumber reg, regNumber base, regNumber index, unsigned scale, int disp)
instruction ins, emitAttr attr, regNumber reg, regNumber base, regNumber index, unsigned scale, cnsval_ssize_t disp)
{
UNATIVE_OFFSET sz;
instrDesc* id = emitNewInstrAmd(attr, disp);
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/src/jit/emitxarch.h
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ void emitInsRMW(instruction inst, emitAttr attr, GenTreeStoreInd* storeInd);

void emitIns_Nop(unsigned size);

void emitIns_I(instruction ins, emitAttr attr, int val);
void emitIns_I(instruction ins, emitAttr attr, cnsval_ssize_t val);

void emitIns_R(instruction ins, emitAttr attr, regNumber reg);

Expand Down Expand Up @@ -409,7 +409,7 @@ void emitIns_R_AR(instruction ins, emitAttr attr, regNumber reg, regNumber base,

void emitIns_R_AI(instruction ins, emitAttr attr, regNumber ireg, ssize_t disp);

void emitIns_AR_R(instruction ins, emitAttr attr, regNumber reg, regNumber base, int disp);
void emitIns_AR_R(instruction ins, emitAttr attr, regNumber reg, regNumber base, cnsval_ssize_t disp);

void emitIns_AI_R(instruction ins, emitAttr attr, regNumber ireg, ssize_t disp);

Expand All @@ -425,7 +425,7 @@ void emitIns_R_ARX(
instruction ins, emitAttr attr, regNumber reg, regNumber base, regNumber index, unsigned scale, int disp);

void emitIns_ARX_R(
instruction ins, emitAttr attr, regNumber reg, regNumber base, regNumber index, unsigned scale, int disp);
instruction ins, emitAttr attr, regNumber reg, regNumber base, regNumber index, unsigned scale, cnsval_ssize_t disp);

void emitIns_I_AX(instruction ins, emitAttr attr, int val, regNumber reg, unsigned mul, int disp);

Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/src/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3310,14 +3310,14 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree)
costSz = 8;
costEx = 2;
}
else if (codeGen->validImmForInstr(INS_add, (target_ssize_t)conVal))
else if (codeGen->validImmForInstr(INS_add, conVal))
{
// Typically included with parent oper
costSz = 2;
costEx = 1;
}
else if (codeGen->validImmForInstr(INS_mov, (target_ssize_t)conVal) &&
codeGen->validImmForInstr(INS_mvn, (target_ssize_t)conVal))
else if (codeGen->validImmForInstr(INS_mov, conVal) &&
codeGen->validImmForInstr(INS_mvn, conVal))
{
// Uses mov or mvn
costSz = 4;
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/src/jit/instr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ void CodeGen::inst_RV_RV_RV(instruction ins,
* Generate a "op icon" instruction.
*/

void CodeGen::inst_IV(instruction ins, int val)
void CodeGen::inst_IV(instruction ins, cnsval_ssize_t val)
{
GetEmitter()->emitIns_I(ins, EA_PTRSIZE, val);
}
Expand All @@ -465,7 +465,7 @@ void CodeGen::inst_IV(instruction ins, int val)
* by 'flags'
*/

void CodeGen::inst_IV_handle(instruction ins, int val)
void CodeGen::inst_IV_handle(instruction ins, cnsval_ssize_t val)
{
GetEmitter()->emitIns_I(ins, EA_HANDLE_CNS_RELOC, val);
}
Expand Down Expand Up @@ -631,9 +631,9 @@ void CodeGen::inst_TT(instruction ins, GenTree* tree, unsigned offs, int shfv, e
assert(offs == 0);
assert(!shfv);
if (tree->IsIconHandle())
inst_IV_handle(ins, (target_ssize_t)tree->AsIntCon()->gtIconVal);
inst_IV_handle(ins, tree->AsIntCon()->gtIconVal);
else
inst_IV(ins, (target_ssize_t)tree->AsIntCon()->gtIconVal);
inst_IV(ins, tree->AsIntCon()->gtIconVal);
break;
#endif

Expand Down
9 changes: 9 additions & 0 deletions src/coreclr/src/jit/target.h
Original file line number Diff line number Diff line change
Expand Up @@ -2013,6 +2013,15 @@ typedef int target_ssize_t;
C_ASSERT(sizeof(target_size_t) == TARGET_POINTER_SIZE);
C_ASSERT(sizeof(target_ssize_t) == TARGET_POINTER_SIZE);

#if defined(TARGET_X86)
typedef ssize_t cnsval_ssize_t;
Copy link
Member

Choose a reason for hiding this comment

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

It's hard to understand why this is specific to target_x86. Why not target_arm or 32-bit targets? Can you add a comment to explain?

typedef size_t cnsval_size_t;
#else
typedef target_ssize_t cnsval_ssize_t;
typedef target_size_t cnsval_size_t;
#endif


/*****************************************************************************/
#endif // TARGET_H_
/*****************************************************************************/
21 changes: 17 additions & 4 deletions src/coreclr/src/tools/Common/JitInterface/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ private enum ImageFileMachine
#if SUPPORT_JIT
private const string JitSupportLibrary = "*";
#else
private const string JitSupportLibrary = "jitinterface";
internal const string JitSupportLibrary = "jitinterface";
#endif

private IntPtr _jit;
Expand Down Expand Up @@ -209,11 +209,11 @@ private void CompileMethodInternal(IMethodNode methodCodeNodeNeedingCode, Method
#endif
}

PublishCode();
PublishCode(codeSize);
PublishROData();
}

private void PublishCode()
private void PublishCode(uint codeSize)
{
var relocs = _codeRelocs.ToArray();
Array.Sort(relocs, (x, y) => (x.Offset - y.Offset));
Expand All @@ -224,7 +224,10 @@ private void PublishCode()

alignment = Math.Max(alignment, _codeAlignment);

var objectData = new ObjectNode.ObjectData(_code,
byte[] actualCodeBytes = _code;
Array.Resize(ref actualCodeBytes, (int)codeSize);

var objectData = new ObjectNode.ObjectData(actualCodeBytes,
relocs,
alignment,
new ISymbolDefinitionNode[] { _methodCodeNode });
Expand Down Expand Up @@ -397,12 +400,19 @@ private void CompileMethodCleanup()
private const int handleMultipler = 8;
private const int handleBase = 0x420000;

#if DEBUG
private static readonly IntPtr s_handleHighBitSet = (sizeof(IntPtr) == 4) ? new IntPtr(0x40000000) : new IntPtr(0x4000000000000000);
#endif

private IntPtr ObjectToHandle(Object obj)
{
IntPtr handle;
if (!_objectToHandle.TryGetValue(obj, out handle))
{
handle = (IntPtr)(handleMultipler * _handleToObject.Count + handleBase);
#if DEBUG
handle = new IntPtr((long)s_handleHighBitSet | (long)handle);
#endif
_handleToObject.Add(obj);
_objectToHandle.Add(obj, handle);
}
Expand All @@ -411,6 +421,9 @@ private IntPtr ObjectToHandle(Object obj)

private Object HandleToObject(IntPtr handle)
{
#if DEBUG
handle = new IntPtr(~(long)s_handleHighBitSet & (long) handle);
#endif
int index = ((int)handle - handleBase) / handleMultipler;
return _handleToObject[index];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ public static void Initialize(
libHandle = NativeLibrary.Load("clrjit_" + GetTargetSpec(target), assembly, searchPath);
}
}
if (libName == CorInfoImpl.JitSupportLibrary)
{
libHandle = NativeLibrary.Load("jitinterface_" + RuntimeInformation.ProcessArchitecture.ToString().ToLowerInvariant(), assembly, searchPath);
}
return libHandle;
});
#else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,8 @@ public override int OffsetFromGCRefMapPos(int pos)
}
}

public override bool IsArgPassedByRef(TypeHandle th) => false;

/// <summary>
/// x86 is special as always
/// DESKTOP BEHAVIOR ret += this.HasThis() ? ArgumentRegisters.GetOffsetOfEdx() : ArgumentRegisters.GetOffsetOfEcx();
Expand Down
Loading