From a0c467bfabc161c37cf60898495a36b9d302a21a Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 11 Feb 2025 23:49:23 +0100 Subject: [PATCH 1/4] Proper eval order for COMMA in impStoreStruct --- src/coreclr/jit/importer.cpp | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index d3de5273c0b613..a465834631f3d8 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -967,9 +967,31 @@ GenTree* Compiler::impStoreStruct(GenTree* store, } else if (src->OperIs(GT_COMMA)) { + GenTree* sideEffectAddressStore = nullptr; + if (store->OperIs(GT_STORE_BLK, GT_STOREIND)) + { + GenTree* addr = store->AsIndir()->Addr(); + if ((addr->gtFlags & GTF_ALL_EFFECT) != 0) + { + if (!addr->OperIs(GT_FIELD_ADDR) || (addr->gtGetOp1()->gtFlags & GTF_ALL_EFFECT) != 0) + { + TempInfo addrikTmp = fgMakeTemp(addr); + sideEffectAddressStore = addrikTmp.store; + store->AsIndir()->Addr() = addrikTmp.load; + } + } + } + if (pAfterStmt) { // Insert op1 after '*pAfterStmt' + if (sideEffectAddressStore != nullptr) + { + Statement* addrStmt = gtNewStmt(sideEffectAddressStore, usedDI); + fgInsertStmtAfter(block, *pAfterStmt, addrStmt); + *pAfterStmt = addrStmt; + } + Statement* newStmt = gtNewStmt(src->AsOp()->gtOp1, usedDI); fgInsertStmtAfter(block, *pAfterStmt, newStmt); *pAfterStmt = newStmt; @@ -977,6 +999,10 @@ GenTree* Compiler::impStoreStruct(GenTree* store, else if (impLastStmt != nullptr) { // Do the side-effect as a separate statement. + if (sideEffectAddressStore != nullptr) + { + impAppendTree(sideEffectAddressStore, curLevel, usedDI); + } impAppendTree(src->AsOp()->gtOp1, curLevel, usedDI); } else @@ -989,6 +1015,10 @@ GenTree* Compiler::impStoreStruct(GenTree* store, gtUpdateNodeSideEffects(store); src->SetAllEffectsFlags(src->AsOp()->gtOp1, src->AsOp()->gtOp2); + if (sideEffectAddressStore != nullptr) + { + src = gtNewOperNode(GT_COMMA, src->TypeGet(), sideEffectAddressStore, src); + } return src; } From 04bc7c15a7b00ae726c57750fae2b8cab13191b9 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 12 Feb 2025 01:56:04 +0100 Subject: [PATCH 2/4] Simplify address store handling in importer.cpp --- src/coreclr/jit/importer.cpp | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index a465834631f3d8..d1daa5753a013b 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -968,18 +968,12 @@ GenTree* Compiler::impStoreStruct(GenTree* store, else if (src->OperIs(GT_COMMA)) { GenTree* sideEffectAddressStore = nullptr; - if (store->OperIs(GT_STORE_BLK, GT_STOREIND)) + if (store->OperIs(GT_STORE_BLK, GT_STOREIND) && + ((store->AsIndir()->Addr()->gtFlags & GTF_ALL_EFFECT) != 0)) { - GenTree* addr = store->AsIndir()->Addr(); - if ((addr->gtFlags & GTF_ALL_EFFECT) != 0) - { - if (!addr->OperIs(GT_FIELD_ADDR) || (addr->gtGetOp1()->gtFlags & GTF_ALL_EFFECT) != 0) - { - TempInfo addrikTmp = fgMakeTemp(addr); - sideEffectAddressStore = addrikTmp.store; - store->AsIndir()->Addr() = addrikTmp.load; - } - } + TempInfo addrikTmp = fgMakeTemp(store->AsIndir()->Addr()); + sideEffectAddressStore = addrikTmp.store; + store->AsIndir()->Addr() = addrikTmp.load; } if (pAfterStmt) From fe1689f19b30e7e9b741f0162079b4b2661b22dd Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 12 Feb 2025 02:10:44 +0100 Subject: [PATCH 3/4] Fix indentation in importer.cpp file --- src/coreclr/jit/importer.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index d1daa5753a013b..c31518775a751f 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -968,8 +968,7 @@ GenTree* Compiler::impStoreStruct(GenTree* store, else if (src->OperIs(GT_COMMA)) { GenTree* sideEffectAddressStore = nullptr; - if (store->OperIs(GT_STORE_BLK, GT_STOREIND) && - ((store->AsIndir()->Addr()->gtFlags & GTF_ALL_EFFECT) != 0)) + if (store->OperIs(GT_STORE_BLK, GT_STOREIND) && ((store->AsIndir()->Addr()->gtFlags & GTF_ALL_EFFECT) != 0)) { TempInfo addrikTmp = fgMakeTemp(store->AsIndir()->Addr()); sideEffectAddressStore = addrikTmp.store; From db4aac3dbfb9c096ea970b91cf5f12bb70748db4 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 12 Feb 2025 11:15:13 +0100 Subject: [PATCH 4/4] Fix typo in variable name addrikTmp --- src/coreclr/jit/importer.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index c31518775a751f..cba7df901f0345 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -970,9 +970,9 @@ GenTree* Compiler::impStoreStruct(GenTree* store, GenTree* sideEffectAddressStore = nullptr; if (store->OperIs(GT_STORE_BLK, GT_STOREIND) && ((store->AsIndir()->Addr()->gtFlags & GTF_ALL_EFFECT) != 0)) { - TempInfo addrikTmp = fgMakeTemp(store->AsIndir()->Addr()); - sideEffectAddressStore = addrikTmp.store; - store->AsIndir()->Addr() = addrikTmp.load; + TempInfo addrTmp = fgMakeTemp(store->AsIndir()->Addr()); + sideEffectAddressStore = addrTmp.store; + store->AsIndir()->Addr() = addrTmp.load; } if (pAfterStmt)