Skip to content

Commit

Permalink
Use the latest base register while accessing the stack (dotnet#38834)
Browse files Browse the repository at this point in the history 
* logging

* the fix

* revert lclvars.cpp changes

* Revert "revert lclvars.cpp changes"

This reverts commit d39af7084687e8fe5e6d4f71674ec84d36a88340.

* wip

* revert lclvars.cpp changes

* deleted inst_RV_ST()

* removing logging, added some asserts

* jit-formatting

* add back case of INS_add and some more asserts

* reset lclvars.cpp

* delete comments and cleanup code

* revert changes inside common.il

* Revert "Disable failing Windows arm32 tests (dotnet#38844)"

This reverts commit 311fd81.

* review comments

* added standard function header
  • Loading branch information
@kunalspathak
kunalspathak authored Jul 8, 2020
1 parent c8f1f2b commit 8988325
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 65 deletions.
2 changes: 0 additions & 2 deletions src/coreclr/src/jit/codegen.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1406,8 +1406,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
void inst_SA_RV(instruction ins, unsigned ofs, regNumber reg, var_types type);
void inst_SA_IV(instruction ins, unsigned ofs, int val, var_types type);

void inst_RV_ST(
instruction ins, regNumber reg, TempDsc* tmp, unsigned ofs, var_types type, emitAttr size = EA_UNKNOWN);
void inst_FS_ST(instruction ins, emitAttr size, TempDsc* tmp, unsigned ofs);

void inst_TT(instruction ins, GenTree* tree, unsigned offs = 0, int shfv = 0, emitAttr size = EA_UNKNOWN);
Expand Down
77 changes: 62 additions & 15 deletions src/coreclr/src/jit/emitarm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3510,11 +3510,22 @@ void emitter::emitIns_S(instruction ins, emitAttr attr, int varx, int offs)
NYI("emitIns_S");
}

/*****************************************************************************
*
* Add an instruction referencing a register and a stack-based local variable.
*/
void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int varx, int offs)
//-------------------------------------------------------------------------------------
// emitIns_R_S: Add an instruction referencing a register and a stack-based local variable.
//
// Arguments:
// ins - The instruction to add.
// attr - Oeration size.
// varx - The variable to generate offset for.
// offs - The offset of variable or field in stack.
// pBaseReg - The base register that is used while calculating the offset. For example, if the offset
// with "stack pointer" can't be encoded in instruction, "frame pointer" can be used to get
// the offset of the field. In such case, pBaseReg will store the "fp".
//
// Return Value:
// The pBaseReg that holds the base register that was used to calculate the offset.
//
void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int varx, int offs, regNumber* pBaseReg)
{
if (ins == INS_mov)
{
Expand Down Expand Up @@ -3547,6 +3558,7 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va
insFormat fmt = IF_NONE;
insFlags sf = INS_FLAGS_NOT_SET;
regNumber reg2;
regNumber baseRegUsed;

/* Figure out the variable's frame position */
int base;
Expand All @@ -3555,6 +3567,10 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va

base = emitComp->lvaFrameAddress(varx, emitComp->funCurrentFunc()->funKind != FUNC_ROOT, &reg2, offs,
CodeGen::instIsFP(ins));
if (pBaseReg != nullptr)
{
*pBaseReg = reg2;
}

disp = base + offs;
undisp = unsigned_abs(disp);
Expand All @@ -3575,8 +3591,8 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va
else
{
regNumber rsvdReg = codeGen->rsGetRsvdReg();
emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ true);
emitIns_R_R(INS_add, EA_4BYTE, rsvdReg, reg2);
emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ true, &baseRegUsed);
emitIns_R_R(INS_add, EA_4BYTE, rsvdReg, baseRegUsed);
emitIns_R_R_I(ins, attr, reg1, rsvdReg, 0);
return;
}
Expand All @@ -3599,8 +3615,11 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va
{
// Load disp into a register
regNumber rsvdReg = codeGen->rsGetRsvdReg();
emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false);
emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false, &baseRegUsed);
fmt = IF_T2_E0;

// Ensure the baseReg calculated is correct.
assert(baseRegUsed == reg2);
}
}
else if (ins == INS_add)
Expand All @@ -3625,7 +3644,10 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va
{
// Load disp into a register
regNumber rsvdReg = codeGen->rsGetRsvdReg();
emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false);
emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false, &baseRegUsed);

// Ensure the baseReg calculated is correct.
assert(baseRegUsed == reg2);
emitIns_R_R_R(ins, attr, reg1, reg2, rsvdReg);
return;
}
Expand Down Expand Up @@ -3662,8 +3684,24 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va
appendToCurIG(id);
}

// generate the offset of &varx + offs into a register
void emitter::emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage)
//-------------------------------------------------------------------------------------
// emitIns_genStackOffset: Generate the offset of &varx + offs into a register
//
// Arguments:
// r - Register in which offset calculation result is stored.
// varx - The variable to generate offset for.
// offs - The offset of variable or field in stack.
// isFloatUsage - True if the instruction being generated is a floating point instruction. This requires using
// floating-point offset restrictions. Note that a variable can be non-float, e.g., struct, but
// accessed as a float local field.
// pBaseReg - The base register that is used while calculating the offset. For example, if the offset with
// "stack pointer" can't be encoded in instruction, "frame pointer" can be used to get the offset
// of the field. In such case, pBaseReg will store the "fp".
//
// Return Value:
// The pBaseReg that holds the base register that was used to calculate the offset.
//
void emitter::emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage, regNumber* pBaseReg)
{
regNumber regBase;
int base;
Expand All @@ -3673,11 +3711,13 @@ void emitter::emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFlo
emitComp->lvaFrameAddress(varx, emitComp->funCurrentFunc()->funKind != FUNC_ROOT, &regBase, offs, isFloatUsage);
disp = base + offs;

emitIns_R_S(INS_movw, EA_4BYTE, r, varx, offs);
emitIns_R_S(INS_movw, EA_4BYTE, r, varx, offs, pBaseReg);

if ((disp & 0xffff) != disp)
{
emitIns_R_S(INS_movt, EA_4BYTE, r, varx, offs);
regNumber regBaseUsedInMovT;
emitIns_R_S(INS_movt, EA_4BYTE, r, varx, offs, &regBaseUsedInMovT);
assert(*pBaseReg == regBaseUsedInMovT);
}
}

Expand Down Expand Up @@ -3708,6 +3748,7 @@ void emitter::emitIns_S_R(instruction ins, emitAttr attr, regNumber reg1, int va
insFormat fmt = IF_NONE;
insFlags sf = INS_FLAGS_NOT_SET;
regNumber reg2;
regNumber baseRegUsed;

/* Figure out the variable's frame position */
int base;
Expand Down Expand Up @@ -3736,7 +3777,10 @@ void emitter::emitIns_S_R(instruction ins, emitAttr attr, regNumber reg1, int va
else
{
regNumber rsvdReg = codeGen->rsGetRsvdReg();
emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ true);
emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ true, &baseRegUsed);

// Ensure the baseReg calculated is correct.
assert(baseRegUsed == reg2);
emitIns_R_R(INS_add, EA_4BYTE, rsvdReg, reg2);
emitIns_R_R_I(ins, attr, reg1, rsvdReg, 0);
return;
Expand All @@ -3758,8 +3802,11 @@ void emitter::emitIns_S_R(instruction ins, emitAttr attr, regNumber reg1, int va
{
// Load disp into a register
regNumber rsvdReg = codeGen->rsGetRsvdReg();
emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false);
emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false, &baseRegUsed);
fmt = IF_T2_E0;

// Ensure the baseReg calculated is correct.
assert(baseRegUsed == reg2);
}
assert((fmt == IF_T1_J2) || (fmt == IF_T2_E0) || (fmt == IF_T2_H0) || (fmt == IF_T2_VLDST) || (fmt == IF_T2_K1));
assert(sf != INS_FLAGS_DONT_CARE);
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/src/jit/emitarm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,11 +267,11 @@ void emitIns_C(instruction ins, emitAttr attr, CORINFO_FIELD_HANDLE fdlHnd, int

void emitIns_S(instruction ins, emitAttr attr, int varx, int offs);

void emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage);
void emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage, regNumber* pBaseReg);

void emitIns_S_R(instruction ins, emitAttr attr, regNumber ireg, int varx, int offs);

void emitIns_R_S(instruction ins, emitAttr attr, regNumber ireg, int varx, int offs);
void emitIns_R_S(instruction ins, emitAttr attr, regNumber ireg, int varx, int offs, regNumber* pBaseReg = nullptr);

void emitIns_S_I(instruction ins, emitAttr attr, int varx, int offs, int val);

Expand Down
34 changes: 0 additions & 34 deletions src/coreclr/src/jit/instr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1308,40 +1308,6 @@ void CodeGen::inst_RV_ST(instruction ins, emitAttr size, regNumber reg, GenTree*
inst_RV_TT(ins, reg, tree, 0, size);
}

void CodeGen::inst_RV_ST(instruction ins, regNumber reg, TempDsc* tmp, unsigned ofs, var_types type, emitAttr size)
{
if (size == EA_UNKNOWN)
{
size = emitActualTypeSize(type);
}

#ifdef TARGET_ARM
switch (ins)
{
case INS_mov:
assert(!"Please call ins_Load(type) to get the load instruction");
break;

case INS_add:
case INS_ldr:
case INS_ldrh:
case INS_ldrb:
case INS_ldrsh:
case INS_ldrsb:
case INS_lea:
case INS_vldr:
GetEmitter()->emitIns_R_S(ins, size, reg, tmp->tdTempNum(), ofs);
break;

default:
assert(!"Default inst_RV_ST case not supported for Arm");
break;
}
#else // !TARGET_ARM
GetEmitter()->emitIns_R_S(ins, size, reg, tmp->tdTempNum(), ofs);
#endif // !TARGET_ARM
}

void CodeGen::inst_mov_RV_ST(regNumber reg, GenTree* tree)
{
/* Figure out the size of the value being loaded */
Expand Down
12 changes: 0 additions & 12 deletions src/coreclr/tests/issues.targets
View file
Original file line number Diff line number Diff line change
Expand Up @@ -560,18 +560,6 @@
<ExcludeList Include="$(XunitTestBinBase)/JIT/jit64/mcc/interop/mcc_i10/*">
<Issue>https://github.com/dotnet/runtime/issues/12979</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/JIT/jit64/mcc/interop/mcc_i74/*">
<Issue>https://github.com/dotnet/runtime/issues/35501</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/JIT/jit64/mcc/interop/mcc_i75/*">
<Issue>https://github.com/dotnet/runtime/issues/35501</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/JIT/jit64/mcc/interop/mcc_i76/*">
<Issue>https://github.com/dotnet/runtime/issues/35501</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/JIT/jit64/mcc/interop/mcc_i77/*">
<Issue>https://github.com/dotnet/runtime/issues/35501</Issue>
</ExcludeList>
</ItemGroup>

<!-- Windows arm64 specific excludes -->
Expand Down

0 comments on commit 8988325

Please sign in to comment.