Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit e226456

Browse files
committed
Fix GT_FIELD_LIST double passed as int
Correctly handle a double field passed in int regs under a `GT_FIELD_LIST`, especially in the case of a `PutArgSplit`. Also, reduce the spew of dump info when `COMPlus_JitStressModeNames` or `COMPlus_JitStressModeNamesNot` are set. Enhance the dumping of `fgArgInfo`. Fix #15325
1 parent ca7448c commit e226456

File tree

8 files changed

+95
-87
lines changed

8 files changed

+95
-87
lines changed

src/jit/codegenarmarch.cpp

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,17 +1000,17 @@ void CodeGen::genPutArgSplit(GenTreePutArgSplit* treeNode)
10001000

10011001
if (source->OperGet() == GT_FIELD_LIST)
10021002
{
1003-
GenTreeFieldList* fieldListPtr = source->AsFieldList();
1004-
10051003
// Evaluate each of the GT_FIELD_LIST items into their register
10061004
// and store their register into the outgoing argument area
1007-
for (unsigned idx = 0; fieldListPtr != nullptr; fieldListPtr = fieldListPtr->Rest(), idx++)
1005+
unsigned regIndex = 0;
1006+
for (GenTreeFieldList* fieldListPtr = source->AsFieldList(); fieldListPtr != nullptr;
1007+
fieldListPtr = fieldListPtr->Rest())
10081008
{
10091009
GenTreePtr nextArgNode = fieldListPtr->gtGetOp1();
10101010
regNumber fieldReg = nextArgNode->gtRegNum;
10111011
genConsumeReg(nextArgNode);
10121012

1013-
if (idx >= treeNode->gtNumRegs)
1013+
if (regIndex >= treeNode->gtNumRegs)
10141014
{
10151015
var_types type = nextArgNode->TypeGet();
10161016
emitAttr attr = emitTypeSize(type);
@@ -1023,14 +1023,32 @@ void CodeGen::genPutArgSplit(GenTreePutArgSplit* treeNode)
10231023
}
10241024
else
10251025
{
1026-
var_types type = treeNode->GetRegType(idx);
1027-
regNumber argReg = treeNode->GetRegNumByIdx(idx);
1026+
var_types type = treeNode->GetRegType(regIndex);
1027+
regNumber argReg = treeNode->GetRegNumByIdx(regIndex);
1028+
if (type == TYP_LONG)
1029+
{
1030+
// We should only see long fields for DOUBLEs passed in 2 integer registers, via bitcast.
1031+
// All other LONGs should have been decomposed.
1032+
// Handle the first INT, and then handle the 2nd below.
1033+
assert(nextArgNode->OperIs(GT_BITCAST));
1034+
type = TYP_INT;
1035+
if (argReg != fieldReg)
1036+
{
1037+
inst_RV_RV(ins_Copy(type), argReg, fieldReg, type);
1038+
}
1039+
// Now set up the next register for the 2nd INT
1040+
argReg = REG_NEXT(argReg);
1041+
regIndex++;
1042+
assert(argReg == treeNode->GetRegNumByIdx(regIndex));
1043+
fieldReg = nextArgNode->AsMultiRegOp()->GetRegNumByIdx(1);
1044+
}
10281045

10291046
// If child node is not already in the register we need, move it
10301047
if (argReg != fieldReg)
10311048
{
10321049
inst_RV_RV(ins_Copy(type), argReg, fieldReg, type);
10331050
}
1051+
regIndex++;
10341052
}
10351053
}
10361054
}

src/jit/compiler.cpp

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3879,10 +3879,6 @@ bool Compiler::compStressCompile(compStressArea stressArea, unsigned weight)
38793879
if ((strStressModeNamesNot != nullptr) &&
38803880
(wcsstr(strStressModeNamesNot, s_compStressModeNames[stressArea]) != nullptr))
38813881
{
3882-
if (verbose)
3883-
{
3884-
printf("JitStressModeNamesNot contains %ws\n", s_compStressModeNames[stressArea]);
3885-
}
38863882
doStress = false;
38873883
goto _done;
38883884
}
@@ -3893,10 +3889,6 @@ bool Compiler::compStressCompile(compStressArea stressArea, unsigned weight)
38933889
{
38943890
if (wcsstr(strStressModeNames, s_compStressModeNames[stressArea]) != nullptr)
38953891
{
3896-
if (verbose)
3897-
{
3898-
printf("JitStressModeNames contains %ws\n", s_compStressModeNames[stressArea]);
3899-
}
39003892
doStress = true;
39013893
goto _done;
39023894
}

src/jit/compiler.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1447,6 +1447,8 @@ class fgArgInfo
14471447

14481448
// Get the late arg for arg at position argIndex. Caller must ensure this position has a late arg.
14491449
GenTreePtr GetLateArg(unsigned argIndex);
1450+
1451+
void Dump(Compiler* compiler);
14501452
};
14511453

14521454
#ifdef DEBUG

src/jit/importer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13337,7 +13337,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
1333713337
!(prefixFlags & PREFIX_TAILCALL_EXPLICIT) && // User hasn't set "tail." prefix yet.
1333813338
verCheckTailCallConstraint(opcode, &resolvedToken,
1333913339
constraintCall ? &constrainedResolvedToken : nullptr,
13340-
true) // Is it legal to do talcall?
13340+
true) // Is it legal to do tailcall?
1334113341
)
1334213342
{
1334313343
// Stress the tailcall.

src/jit/lower.cpp

Lines changed: 38 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1488,44 +1488,12 @@ void Lowering::LowerArg(GenTreeCall* call, GenTreePtr* ppArg)
14881488
#ifdef _TARGET_ARMARCH_
14891489
if (call->IsVarargs() || comp->opts.compUseSoftFP)
14901490
{
1491-
14921491
// For vararg call or on armel, reg args should be all integer.
1493-
// Insert a copy to move float value to integer register.
1494-
if (varTypeIsFloating(type))
1492+
// Insert copies as needed to move float value to integer register.
1493+
GenTree* newNode = LowerFloatArg(ppArg, info);
1494+
if (newNode != nullptr)
14951495
{
1496-
#ifdef _TARGET_ARM_
1497-
#ifdef DEBUG
1498-
if (type == TYP_DOUBLE)
1499-
{
1500-
unsigned numRegs = info->numRegs;
1501-
regNumber regCurr = info->regNum;
1502-
assert(numRegs % 2 == 0);
1503-
for (unsigned i = 0; i < numRegs;)
1504-
{
1505-
regNumber regNext = REG_NEXT(regCurr);
1506-
// double type arg regs can only be either r0:r1 or r2:r3.
1507-
assert((regCurr == REG_R0 && regNext == REG_R1) || (regCurr == REG_R2 && regNext == REG_R3));
1508-
1509-
i += 2;
1510-
regCurr = REG_NEXT(regNext);
1511-
}
1512-
}
1513-
#endif // DEBUG
1514-
#endif // _TARGET_ARM_
1515-
1516-
GenTreePtr intArg = LowerFloatArg(arg, info);
1517-
if (intArg != nullptr)
1518-
{
1519-
if (intArg != arg)
1520-
{
1521-
ReplaceArgWithPutArgOrBitcast(ppArg, intArg);
1522-
arg = intArg;
1523-
info->node = intArg;
1524-
}
1525-
1526-
// update local variables.
1527-
type = arg->TypeGet();
1528-
}
1496+
type = newNode->TypeGet();
15291497
}
15301498
}
15311499
#endif // _TARGET_ARMARCH_
@@ -1544,7 +1512,7 @@ void Lowering::LowerArg(GenTreeCall* call, GenTreePtr* ppArg)
15441512

15451513
#ifdef _TARGET_ARMARCH_
15461514
//------------------------------------------------------------------------
1547-
// LowerFloatArg: Lower the float call argument on the arm platform.
1515+
// LowerFloatArg: Lower float call arguments on the arm platform.
15481516
//
15491517
// Arguments:
15501518
// arg - The arg node
@@ -1555,8 +1523,13 @@ void Lowering::LowerArg(GenTreeCall* call, GenTreePtr* ppArg)
15551523
// return arg if there was in place transformation;
15561524
// return a new tree if the root was changed.
15571525
//
1558-
GenTree* Lowering::LowerFloatArg(GenTree* arg, fgArgTabEntry* info)
1526+
// Notes:
1527+
// This must handle scalar float arguments as well as GT_FIELD_LISTs
1528+
// with floating point fields.
1529+
//
1530+
GenTree* Lowering::LowerFloatArg(GenTree** pArg, fgArgTabEntry* info)
15591531
{
1532+
GenTree* arg = *pArg;
15601533
if (info->regNum != REG_STK)
15611534
{
15621535
if (arg->OperIsFieldList())
@@ -1565,32 +1538,44 @@ GenTree* Lowering::LowerFloatArg(GenTree* arg, fgArgTabEntry* info)
15651538
regNumber currRegNumber = info->regNum;
15661539

15671540
// Transform fields that are passed as registers in place.
1568-
for (unsigned i = 0; i < info->numRegs; ++i)
1541+
unsigned fieldRegCount;
1542+
for (unsigned i = 0; i < info->numRegs; i += fieldRegCount)
15691543
{
15701544
assert(currListNode != nullptr);
1571-
GenTree* node = currListNode->Current();
1572-
GenTree* intNode = LowerFloatArgReg(node, currRegNumber);
1573-
assert(intNode != nullptr);
1545+
GenTree* node = currListNode->Current();
1546+
if (varTypeIsFloating(node))
1547+
{
1548+
GenTree* intNode = LowerFloatArgReg(node, currRegNumber);
1549+
assert(intNode != nullptr);
15741550

1575-
ReplaceArgWithPutArgOrBitcast(currListNode->pCurrent(), intNode);
1576-
currListNode->ChangeType(intNode->TypeGet());
1551+
ReplaceArgWithPutArgOrBitcast(currListNode->pCurrent(), intNode);
1552+
currListNode->ChangeType(intNode->TypeGet());
1553+
}
15771554

1578-
currListNode = currListNode->Rest();
1579-
currRegNumber = REG_NEXT(currRegNumber);
1555+
if (node->TypeGet() == TYP_DOUBLE)
1556+
{
1557+
currRegNumber = REG_NEXT(REG_NEXT(currRegNumber));
1558+
fieldRegCount = 2;
1559+
}
1560+
else
1561+
{
1562+
currRegNumber = REG_NEXT(currRegNumber);
1563+
fieldRegCount = 1;
1564+
}
1565+
currListNode = currListNode->Rest();
15801566
}
15811567
// List fields were replaced in place.
15821568
return arg;
15831569
}
1584-
else
1570+
else if (varTypeIsFloating(arg))
15851571
{
1586-
return LowerFloatArgReg(arg, info->regNum);
1572+
GenTree* intNode = LowerFloatArgReg(arg, info->regNum);
1573+
assert(intNode != nullptr);
1574+
ReplaceArgWithPutArgOrBitcast(pArg, intNode);
1575+
return *pArg;
15871576
}
15881577
}
1589-
else
1590-
{
1591-
// Do not change stack nodes.
1592-
return nullptr;
1593-
}
1578+
return nullptr;
15941579
}
15951580

15961581
//------------------------------------------------------------------------

src/jit/lower.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ class Lowering : public Phase
155155
GenTree* NewPutArg(GenTreeCall* call, GenTreePtr arg, fgArgTabEntryPtr info, var_types type);
156156
void LowerArg(GenTreeCall* call, GenTreePtr* ppTree);
157157
#ifdef _TARGET_ARMARCH_
158-
GenTree* LowerFloatArg(GenTree* arg, fgArgTabEntry* info);
158+
GenTree* LowerFloatArg(GenTree** pArg, fgArgTabEntry* info);
159159
GenTree* LowerFloatArgReg(GenTree* arg, regNumber regNum);
160160
#endif
161161

src/jit/lsraarmarch.cpp

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -737,24 +737,29 @@ void LinearScan::TreeNodeInfoInitPutArgSplit(GenTreePutArgSplit* argNode)
737737
// 1. Consume all of the items in the GT_FIELD_LIST (source)
738738
// 2. Store to target slot and move to target registers (destination) from source
739739
//
740-
unsigned slotCount = 0;
740+
unsigned sourceRegCount = 0;
741741

742742
// To avoid redundant moves, have the argument operand computed in the
743743
// register in which the argument is passed to the call.
744-
GenTreeFieldList* fieldListPtr = putArgChild->AsFieldList();
745-
for (unsigned idx = 0; fieldListPtr != nullptr; fieldListPtr = fieldListPtr->Rest(), idx++)
744+
745+
for (GenTreeFieldList* fieldListPtr = putArgChild->AsFieldList(); fieldListPtr != nullptr;
746+
fieldListPtr = fieldListPtr->Rest())
746747
{
747-
if (idx < argNode->gtNumRegs)
748-
{
749-
GenTreePtr node = fieldListPtr->gtGetOp1();
750-
node->gtLsraInfo.setSrcCandidates(this, genRegMask((regNumber)((unsigned)argReg + idx)));
751-
}
752-
else
748+
GenTreePtr node = fieldListPtr->gtGetOp1();
749+
assert(!node->isContained());
750+
unsigned currentRegCount = node->gtLsraInfo.dstCount;
751+
regMaskTP sourceMask = RBM_NONE;
752+
if (sourceRegCount < argNode->gtNumRegs)
753753
{
754-
slotCount++;
754+
for (unsigned regIndex = 0; regIndex < currentRegCount; regIndex++)
755+
{
756+
sourceMask |= genRegMask((regNumber)((unsigned)argReg + sourceRegCount + regIndex));
757+
}
758+
node->gtLsraInfo.setSrcCandidates(this, sourceMask);
755759
}
760+
sourceRegCount += currentRegCount;
756761
}
757-
argNode->gtLsraInfo.srcCount = argNode->gtNumRegs + slotCount;
762+
argNode->gtLsraInfo.srcCount = sourceRegCount;
758763
assert(putArgChild->isContained());
759764
}
760765
else

src/jit/morph.cpp

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -898,6 +898,7 @@ unsigned UpdateGT_LISTFlags(GenTreePtr tree)
898898
void fgArgTabEntry::Dump()
899899
{
900900
printf("fgArgTabEntry[arg %u", argNum);
901+
printf(" %d.%s", node->gtTreeID, GenTree::OpName(node->gtOper));
901902
if (regNum != REG_STK)
902903
{
903904
printf(", %s, regs=%u", getRegName(regNum), numRegs);
@@ -2133,6 +2134,17 @@ void fgArgInfo::SortArgs()
21332134
argsSorted = true;
21342135
}
21352136

2137+
#ifdef DEBUG
2138+
void fgArgInfo::Dump(Compiler* compiler)
2139+
{
2140+
for (unsigned curInx = 0; curInx < ArgCount(); curInx++)
2141+
{
2142+
fgArgTabEntryPtr curArgEntry = ArgTable()[curInx];
2143+
curArgEntry->Dump();
2144+
}
2145+
}
2146+
#endif
2147+
21362148
//------------------------------------------------------------------------------
21372149
// fgMakeTmpArgNode : This function creates a tmp var only if needed.
21382150
// We need this to be done in order to enforce ordering
@@ -4585,15 +4597,9 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call)
45854597
#ifdef DEBUG
45864598
if (verbose)
45874599
{
4588-
fgArgInfoPtr argInfo = call->fgArgInfo;
4589-
for (unsigned curInx = 0; curInx < argInfo->ArgCount(); curInx++)
4590-
{
4591-
fgArgTabEntryPtr curArgEntry = argInfo->ArgTable()[curInx];
4592-
curArgEntry->Dump();
4593-
}
4600+
call->fgArgInfo->Dump(this);
45944601
}
45954602
#endif
4596-
45974603
return call;
45984604
}
45994605
#ifdef _PREFAST_

0 commit comments

Comments
 (0)