Skip to content

Commit

Permalink
add comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Sergey Andreenko committed Jan 6, 2020
1 parent 0f73f4e commit efba255
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 10 deletions.
6 changes: 6 additions & 0 deletions src/coreclr/src/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14878,6 +14878,12 @@ GenTree* Compiler::gtNewTempAssign(
CORINFO_CLASS_HANDLE structHnd = gtGetStructHandleIfPresent(val);
if (varTypeIsStruct(valTyp) && ((structHnd != NO_CLASS_HANDLE) || (varTypeIsSIMD(valTyp))))
{
// There are 3 possibilitiess here:
// 1) `pAfterStmt != nullptr`, so we know where to insert a new stmt if we need to;
// 2) we are during importation, so we can spill stmt to `impStmtList`;
// 3) we don't have `pAfterStmt` and we are not during importation, then we probably don't need to create new
// stmt or
// we will do magic with commas in `impAssignStructPtr`.
// The struct value may be be a child of a GT_COMMA.
GenTree* valx = val->gtEffectiveVal(/*commaOnly*/ true);

Expand Down
29 changes: 19 additions & 10 deletions src/coreclr/src/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1140,6 +1140,24 @@ GenTree* Compiler::impAssignStruct(GenTree* dest,
//
// Notes:
// Temp assignments may be appended to impStmtList if spilling is necessary.
// sandreenko:
// 1) The method has imp prefix, but it is a lie, because it can be called from
// `gtNewTempAssign` that can be called, for example, from `PerformCSE`.
// 2) the method could create a new statement in `src->gtOper == GT_MKREFANY` case, that is only valid during
// import phase,
// and in `src->gtOper == GT_COMMA` case. Other cases do not need `asgPlace` and do not create any stms.
// Comma case needs to spill COMMA operands, because some phases do not support struct assignments with COMMAs on
// the rhs.
// If `asgPlace.useStms == true` then it will spill `COMMA->gtOp1` there, otherwise, if we are during importation,
// it will spill `COMMA->gtOp1`
// into `impStmtList`. Otherwise it will sink the assignment below the COMMA. The third case is ugly also because
// it violates "Return Value" contract for that method. It returns not the tree that should be appended, but a
// comma tree that is already in
// the stmt list and the callers should check for this case as `PerfomCSE` does.
// If we get rid of creating new Stmt in that method for these 2 cases we will be able to get rid of `asgPlace`
// argument.
//
//

GenTree* Compiler::impAssignStructPtr(GenTree* destAddr,
GenTree* src,
Expand Down Expand Up @@ -1310,16 +1328,7 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr,

// append the assign of the pointer value
GenTree* asg = gtNewAssignNode(ptrSlot, src->AsOp()->gtOp1);
if (asgPlace.useStmts)
{
Statement* newStmt = gtNewStmt(asg, ilOffset);
fgInsertStmtAfter(asgPlace.block, *asgPlace.pAfterStmt, newStmt);
*asgPlace.pAfterStmt = newStmt;
}
else
{
impAppendTree(asg, asgPlace.spillLevel, ilOffset);
}
impAppendTree(asg, asgPlace.spillLevel, ilOffset);

// return the assign of the type value, to be appended
return gtNewAssignNode(typeSlot, src->AsOp()->gtOp2);
Expand Down

0 comments on commit efba255

Please sign in to comment.