Skip to content

Commit c04cf0c

Browse files
Split and simplify fgMorphOneAsgBlockOp (#76793)
* Delete gtNewCpObjNode * Import INITOBJ of primitives as STIND No point in creating block nodes for simple types. * Retype local FP stores too Avoids a couple regressions. * Handle a bit of what OneAsg morphing does in block morphing * Slim down fgMorphOneAsgBlockOp A zero-diff checkpoint. * Slim down fgMorphOneAsgBlockOp, part 2 A zero-diff checkpoint. * Slim down fgMorphOneAsgBlockOp, final part 3 small regressions across all of SPMI due to less aggressive CSEing ("LCL_FLD" is costed cheaper than "IND(ADDR(LCL_VAR))"). * Don't call OneAsg for init blocks * Delete assertion killing dead code PrepareDst will have killed assertions already. * Don't call gtSetObjGcInfo
1 parent 5bc25e6 commit c04cf0c

File tree

8 files changed

+223
-473
lines changed

8 files changed

+223
-473
lines changed

src/coreclr/jit/compiler.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2373,8 +2373,6 @@ class Compiler
23732373
GenTree* gtNewStructVal(ClassLayout* layout, GenTree* addr);
23742374
GenTree* gtNewBlockVal(GenTree* addr, unsigned size);
23752375

2376-
GenTree* gtNewCpObjNode(GenTree* dst, GenTree* src, CORINFO_CLASS_HANDLE structHnd, bool isVolatile);
2377-
23782376
GenTreeCall* gtNewCallNode(gtCallTypes callType,
23792377
CORINFO_METHOD_HANDLE handle,
23802378
var_types type,

src/coreclr/jit/gentree.cpp

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -7692,42 +7692,6 @@ GenTree* Compiler::gtNewBlockVal(GenTree* addr, unsigned size)
76927692
return blkNode;
76937693
}
76947694

7695-
// Creates a new assignment node for a CpObj.
7696-
// Parameters (exactly the same as MSIL CpObj):
7697-
//
7698-
// dstAddr - The target to copy the struct to
7699-
// srcAddr - The source to copy the struct from
7700-
// structHnd - A class token that represents the type of object being copied. May be null
7701-
// if FEATURE_SIMD is enabled and the source has a SIMD type.
7702-
// isVolatile - Is this marked as volatile memory?
7703-
7704-
GenTree* Compiler::gtNewCpObjNode(GenTree* dstAddr, GenTree* srcAddr, CORINFO_CLASS_HANDLE structHnd, bool isVolatile)
7705-
{
7706-
ClassLayout* layout = typGetObjLayout(structHnd);
7707-
GenTree* lhs = gtNewStructVal(layout, dstAddr);
7708-
GenTree* src = gtNewStructVal(layout, srcAddr);
7709-
7710-
if (lhs->OperIs(GT_OBJ))
7711-
{
7712-
GenTreeObj* lhsObj = lhs->AsObj();
7713-
#if DEBUG
7714-
// Codegen for CpObj assumes that we cannot have a struct with GC pointers whose size is not a multiple
7715-
// of the register size. The EE currently does not allow this to ensure that GC pointers are aligned
7716-
// if the struct is stored in an array. Note that this restriction doesn't apply to stack-allocated objects:
7717-
// they are never stored in arrays. We should never get to this method with stack-allocated objects since they
7718-
// are never copied so we don't need to exclude them from the assert below.
7719-
// Let's assert it just to be safe.
7720-
ClassLayout* layout = lhsObj->GetLayout();
7721-
unsigned size = layout->GetSize();
7722-
assert((layout->GetGCPtrCount() == 0) || (roundUp(size, REGSIZE_BYTES) == size));
7723-
#endif
7724-
gtSetObjGcInfo(lhsObj);
7725-
}
7726-
7727-
GenTree* result = gtNewBlkOpNode(lhs, src, isVolatile, true);
7728-
return result;
7729-
}
7730-
77317695
//------------------------------------------------------------------------
77327696
// FixupInitBlkValue: Fixup the init value for an initBlk operation
77337697
//

src/coreclr/jit/gentree.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1812,6 +1812,8 @@ struct GenTree
18121812
// The returned pointer might be nullptr if the node is not binary, or if non-null op2 is not required.
18131813
inline GenTree* gtGetOp2IfPresent() const;
18141814

1815+
inline GenTree*& Data();
1816+
18151817
bool TryGetUse(GenTree* operand, GenTree*** pUse);
18161818

18171819
bool TryGetUse(GenTree* operand)
@@ -8937,6 +8939,12 @@ inline GenTree* GenTree::gtGetOp2IfPresent() const
89378939
return op2;
89388940
}
89398941

8942+
inline GenTree*& GenTree::Data()
8943+
{
8944+
assert(OperIsStore() && (OperIsIndir() || OperIsLocal()));
8945+
return OperIsLocalStore() ? AsLclVarCommon()->Data() : AsIndir()->Data();
8946+
}
8947+
89408948
inline GenTree* GenTree::gtEffectiveVal(bool commaOnly /* = false */)
89418949
{
89428950
GenTree* effectiveVal = this;

src/coreclr/jit/importer.cpp

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8648,9 +8648,11 @@ void Compiler::impImportBlockCode(BasicBlock* block)
86488648
case CEE_STIND_R8:
86498649
lclTyp = TYP_DOUBLE;
86508650
goto STIND;
8651-
STIND:
86528651

8652+
STIND:
86538653
op2 = impPopStack().val; // value to store
8654+
8655+
STIND_VALUE:
86548656
op1 = impPopStack().val; // address to store to
86558657

86568658
// you can indirect off of a TYP_I_IMPL (if we are in C) or a BYREF
@@ -10861,24 +10863,17 @@ void Compiler::impImportBlockCode(BasicBlock* block)
1086110863

1086210864
JITDUMP(" %08X", resolvedToken.token);
1086310865

10864-
op2 = gtNewIconNode(0); // Value
10865-
op1 = impPopStack().val; // Dest
10866-
10867-
if (eeIsValueClass(resolvedToken.hClass))
10868-
{
10869-
op1 = gtNewStructVal(typGetObjLayout(resolvedToken.hClass), op1);
10870-
if (op1->OperIs(GT_OBJ))
10871-
{
10872-
gtSetObjGcInfo(op1->AsObj());
10873-
}
10874-
}
10875-
else
10866+
lclTyp = JITtype2varType(info.compCompHnd->asCorInfoType(resolvedToken.hClass));
10867+
if (lclTyp != TYP_STRUCT)
1087610868
{
10877-
size = info.compCompHnd->getClassSize(resolvedToken.hClass);
10878-
assert(size == TARGET_POINTER_SIZE);
10879-
op1 = gtNewBlockVal(op1, size);
10869+
op2 = gtNewZeroConNode(genActualType(lclTyp));
10870+
goto STIND_VALUE;
1088010871
}
1088110872

10873+
op1 = impPopStack().val;
10874+
op1 = gtNewStructVal(typGetObjLayout(resolvedToken.hClass), op1);
10875+
op2 = gtNewIconNode(0);
10876+
1088210877
op1 = gtNewBlkOpNode(op1, op2, (prefixFlags & PREFIX_VOLATILE) != 0, false);
1088310878
goto SPILL_APPEND;
1088410879

@@ -10909,7 +10904,6 @@ void Compiler::impImportBlockCode(BasicBlock* block)
1090910904
op1->gtFlags |= GTF_BLK_VOLATILE;
1091010905
}
1091110906
}
10912-
1091310907
goto SPILL_APPEND;
1091410908

1091510909
case CEE_CPBLK:
@@ -10936,11 +10930,10 @@ void Compiler::impImportBlockCode(BasicBlock* block)
1093610930
op1->gtFlags |= GTF_BLK_VOLATILE;
1093710931
}
1093810932
}
10939-
1094010933
goto SPILL_APPEND;
1094110934

1094210935
case CEE_CPOBJ:
10943-
10936+
{
1094410937
assertImp(sz == sizeof(unsigned));
1094510938

1094610939
_impResolveToken(CORINFO_TOKENKIND_Class);
@@ -10960,10 +10953,19 @@ void Compiler::impImportBlockCode(BasicBlock* block)
1096010953
goto STIND;
1096110954
}
1096210955

10963-
op2 = impPopStack().val; // Src
10964-
op1 = impPopStack().val; // Dest
10965-
op1 = gtNewCpObjNode(op1, op2, resolvedToken.hClass, ((prefixFlags & PREFIX_VOLATILE) != 0));
10956+
op2 = impPopStack().val; // Src addr
10957+
op1 = impPopStack().val; // Dest addr
10958+
10959+
ClassLayout* layout = typGetObjLayout(resolvedToken.hClass);
10960+
op1 = gtNewStructVal(layout, op1);
10961+
op2 = gtNewStructVal(layout, op2);
10962+
if (op1->OperIs(GT_OBJ))
10963+
{
10964+
gtSetObjGcInfo(op1->AsObj());
10965+
}
10966+
op1 = gtNewBlkOpNode(op1, op2, ((prefixFlags & PREFIX_VOLATILE) != 0), /* isCopyBlock */ true);
1096610967
goto SPILL_APPEND;
10968+
}
1096710969

1096810970
case CEE_STOBJ:
1096910971
{

src/coreclr/jit/lower.cpp

Lines changed: 87 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -3510,6 +3510,8 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
35103510
DISPTREERANGE(BlockRange(), lclStore);
35113511
JITDUMP("\n");
35123512

3513+
TryRetypingFloatingPointStoreToIntegerStore(lclStore);
3514+
35133515
GenTree* src = lclStore->gtGetOp1();
35143516
LclVarDsc* varDsc = comp->lvaGetDesc(lclStore);
35153517
const bool srcIsMultiReg = src->IsMultiRegNode();
@@ -7211,6 +7213,8 @@ void Lowering::LowerStoreIndirCommon(GenTreeStoreInd* ind)
72117213
{
72127214
assert(ind->TypeGet() != TYP_STRUCT);
72137215

7216+
TryRetypingFloatingPointStoreToIntegerStore(ind);
7217+
72147218
#if defined(TARGET_ARM64)
72157219
// Verify containment safety before creating an LEA that must be contained.
72167220
//
@@ -7239,51 +7243,6 @@ void Lowering::LowerStoreIndirCommon(GenTreeStoreInd* ind)
72397243
}
72407244
#endif
72417245

7242-
if (varTypeIsFloating(ind) && ind->Data()->IsCnsFltOrDbl())
7243-
{
7244-
// Optimize *x = DCON to *x = ICON which can be slightly faster and/or smaller.
7245-
GenTree* data = ind->Data();
7246-
double dblCns = data->AsDblCon()->DconValue();
7247-
ssize_t intCns = 0;
7248-
var_types type = TYP_UNKNOWN;
7249-
// XARCH: we can always contain the immediates.
7250-
// ARM64: zero can always be contained, other cases will use immediates from the data
7251-
// section and it is not a clear win to switch them to inline integers.
7252-
// ARM: FP constants are assembled from integral ones, so it is always profitable
7253-
// to directly use the integers as it avoids the int -> float conversion.
7254-
CLANG_FORMAT_COMMENT_ANCHOR;
7255-
7256-
#if defined(TARGET_XARCH) || defined(TARGET_ARM)
7257-
bool shouldSwitchToInteger = true;
7258-
#else // TARGET_ARM64
7259-
bool shouldSwitchToInteger = !data->IsCnsNonZeroFltOrDbl();
7260-
#endif
7261-
7262-
if (shouldSwitchToInteger)
7263-
{
7264-
if (ind->TypeIs(TYP_FLOAT))
7265-
{
7266-
float fltCns = static_cast<float>(dblCns); // should be a safe round-trip
7267-
intCns = static_cast<ssize_t>(*reinterpret_cast<INT32*>(&fltCns));
7268-
type = TYP_INT;
7269-
}
7270-
#ifdef TARGET_64BIT
7271-
else
7272-
{
7273-
assert(ind->TypeIs(TYP_DOUBLE));
7274-
intCns = static_cast<ssize_t>(*reinterpret_cast<INT64*>(&dblCns));
7275-
type = TYP_LONG;
7276-
}
7277-
#endif
7278-
}
7279-
7280-
if (type != TYP_UNKNOWN)
7281-
{
7282-
data->BashToConst(intCns, type);
7283-
ind->ChangeType(type);
7284-
}
7285-
}
7286-
72877246
LowerStoreIndir(ind);
72887247
}
72897248
}
@@ -7514,6 +7473,89 @@ bool Lowering::TryTransformStoreObjAsStoreInd(GenTreeBlk* blkNode)
75147473
return true;
75157474
}
75167475

7476+
//------------------------------------------------------------------------
7477+
// TryRetypingFloatingPointStoreToIntegerStore: Retype an FP memory store.
7478+
//
7479+
// On some targets, integer stores are cheaper and/or smaller than their
7480+
// floating-point counterparts, because, e. g., integer immediates can be
7481+
// encoded inline while FP ones need to be loaded from the data section.
7482+
//
7483+
// Arguments:
7484+
// store - The store node
7485+
//
7486+
void Lowering::TryRetypingFloatingPointStoreToIntegerStore(GenTree* store)
7487+
{
7488+
assert(store->OperIsStore() && !store->OperIsAtomicOp());
7489+
7490+
if (!varTypeIsFloating(store))
7491+
{
7492+
return;
7493+
}
7494+
7495+
// We only want to transform memory stores, not definitions of candidate locals.
7496+
//
7497+
if (store->OperIs(GT_STORE_LCL_VAR) && !comp->lvaGetDesc(store->AsLclVar())->lvDoNotEnregister)
7498+
{
7499+
return;
7500+
}
7501+
7502+
GenTree* data = store->Data();
7503+
assert(store->TypeGet() == data->TypeGet());
7504+
7505+
// Optimize *x = DCON to *x = ICON which can be slightly faster and/or smaller.
7506+
//
7507+
if (data->IsCnsFltOrDbl())
7508+
{
7509+
double dblCns = data->AsDblCon()->DconValue();
7510+
ssize_t intCns = 0;
7511+
var_types type = TYP_UNKNOWN;
7512+
// XARCH: we can always contain the immediates.
7513+
// ARM64: zero can always be contained, other cases will use immediates from the data
7514+
// section and it is not a clear win to switch them to inline integers.
7515+
// ARM: FP constants are assembled from integral ones, so it is always profitable
7516+
// to directly use the integers as it avoids the int -> float conversion.
7517+
CLANG_FORMAT_COMMENT_ANCHOR;
7518+
7519+
#if defined(TARGET_XARCH) || defined(TARGET_ARM)
7520+
bool shouldSwitchToInteger = true;
7521+
#else // TARGET_ARM64 || TARGET_LOONGARCH64
7522+
bool shouldSwitchToInteger = FloatingPointUtils::isPositiveZero(dblCns);
7523+
#endif
7524+
7525+
if (shouldSwitchToInteger)
7526+
{
7527+
if (store->TypeIs(TYP_FLOAT))
7528+
{
7529+
float fltCns = static_cast<float>(dblCns);
7530+
intCns = *reinterpret_cast<INT32*>(&fltCns);
7531+
type = TYP_INT;
7532+
}
7533+
#ifdef TARGET_64BIT
7534+
else
7535+
{
7536+
assert(store->TypeIs(TYP_DOUBLE));
7537+
intCns = *reinterpret_cast<INT64*>(&dblCns);
7538+
type = TYP_LONG;
7539+
}
7540+
#endif
7541+
}
7542+
7543+
if (type != TYP_UNKNOWN)
7544+
{
7545+
data->BashToConst(intCns, type);
7546+
7547+
assert(!store->OperIsLocalStore() || comp->lvaGetDesc(store->AsLclVarCommon())->lvDoNotEnregister);
7548+
if (store->OperIs(GT_STORE_LCL_VAR))
7549+
{
7550+
store->SetOper(GT_STORE_LCL_FLD);
7551+
store->AsLclFld()->SetLclOffs(0);
7552+
store->AsLclFld()->SetLayout(nullptr);
7553+
}
7554+
store->ChangeType(type);
7555+
}
7556+
}
7557+
}
7558+
75177559
#ifdef FEATURE_SIMD
75187560
//----------------------------------------------------------------------------------------------
75197561
// Lowering::LowerSIMD: Perform containment analysis for a SIMD intrinsic node.

src/coreclr/jit/lower.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,8 @@ class Lowering final : public Phase
328328

329329
bool TryTransformStoreObjAsStoreInd(GenTreeBlk* blkNode);
330330

331+
void TryRetypingFloatingPointStoreToIntegerStore(GenTree* store);
332+
331333
GenTree* LowerSwitch(GenTree* node);
332334
bool TryLowerSwitchToBitTest(
333335
BasicBlock* jumpTable[], unsigned jumpCount, unsigned targetCount, BasicBlock* bbSwitch, GenTree* switchValue);

0 commit comments

Comments
 (0)