From 05c201c675aab1897b3c3eb77f8e0689e49afdb6 Mon Sep 17 00:00:00 2001 From: Sergey Date: Thu, 15 Jul 2021 03:00:47 -0700 Subject: [PATCH 1/2] Fix `TryTransformStoreObjAsStoreInd` optimization. --- src/coreclr/jit/gentree.h | 2 +- src/coreclr/jit/lower.cpp | 9 +++++---- src/coreclr/jit/lowerarmarch.cpp | 2 ++ src/coreclr/jit/lowerxarch.cpp | 2 ++ 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 4b7374573ef735..dd04a63789e58c 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1251,7 +1251,7 @@ struct GenTree return OperIsInitVal(OperGet()); } - bool IsConstInitVal() + bool IsConstInitVal() const { return (gtOper == GT_CNS_INT) || (OperIsInitVal() && (gtGetOp1()->gtOper == GT_CNS_INT)); } diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index a5b0ba16fdc6c7..c5b6a75ef2f17c 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -6730,7 +6730,7 @@ void Lowering::LowerBlockStoreCommon(GenTreeBlk* blkNode) bool Lowering::TryTransformStoreObjAsStoreInd(GenTreeBlk* blkNode) { assert(blkNode->OperIs(GT_STORE_BLK, GT_STORE_DYN_BLK, GT_STORE_OBJ)); - if (comp->opts.OptimizationEnabled()) + if (!comp->opts.OptimizationEnabled()) { return false; } @@ -6751,7 +6751,9 @@ bool Lowering::TryTransformStoreObjAsStoreInd(GenTreeBlk* blkNode) { return false; } - if (varTypeIsSIMD(regType)) + + GenTree* src = blkNode->Data(); + if (varTypeIsSIMD(regType) && src->IsConstInitVal()) { // TODO-CQ: support STORE_IND SIMD16(SIMD16, CNT_INT 0). return false; @@ -6764,13 +6766,12 @@ bool Lowering::TryTransformStoreObjAsStoreInd(GenTreeBlk* blkNode) return false; } - GenTree* src = blkNode->Data(); if (src->OperIsInitVal() && !src->IsConstInitVal()) { return false; } - if (varTypeIsSmall(regType) && !src->IsConstInitVal()) + if (varTypeIsSmall(regType) && !src->IsConstInitVal() && !src->IsLocal()) { // source operand INDIR will use a widening instruction // and generate worse code, like `movzx` instead of `mov` diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 13af2eaa280f79..89347328ac9448 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -306,6 +306,8 @@ void Lowering::LowerBlockStore(GenTreeBlk* blkNode) { // TODO-1stClassStructs: for now we can't work with STORE_BLOCK source in register. const unsigned srcLclNum = src->AsLclVar()->GetLclNum(); + INDEBUG(const LclVarDsc* lvlVar = comp->lvaGetDesc(srcLclNum);); + assert(comp->lvaVarDoNotEnregister(srcLclNum) || lvlVar->HasGCPtr()); comp->lvaSetVarDoNotEnregister(srcLclNum DEBUGARG(Compiler::DNER_BlockOp)); } diff --git a/src/coreclr/jit/lowerxarch.cpp b/src/coreclr/jit/lowerxarch.cpp index 43c0df62042365..d4a36411995490 100644 --- a/src/coreclr/jit/lowerxarch.cpp +++ b/src/coreclr/jit/lowerxarch.cpp @@ -280,6 +280,8 @@ void Lowering::LowerBlockStore(GenTreeBlk* blkNode) { // TODO-1stClassStructs: for now we can't work with STORE_BLOCK source in register. const unsigned srcLclNum = src->AsLclVar()->GetLclNum(); + INDEBUG(const LclVarDsc* lvlVar = comp->lvaGetDesc(srcLclNum);); + assert(comp->lvaVarDoNotEnregister(srcLclNum) || lvlVar->HasGCPtr()); comp->lvaSetVarDoNotEnregister(srcLclNum DEBUGARG(Compiler::DNER_BlockOp)); } From ab28a4e81a1c2a839d574d680c2dccb3d2ae5686 Mon Sep 17 00:00:00 2001 From: Sergey Date: Thu, 15 Jul 2021 09:51:59 -0700 Subject: [PATCH 2/2] Disable the assert. It was failing on DevDiv_280120 arm32 linux, did not repro with an altjit. --- src/coreclr/jit/lowerarmarch.cpp | 2 -- src/coreclr/jit/lowerxarch.cpp | 2 -- 2 files changed, 4 deletions(-) diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 89347328ac9448..13af2eaa280f79 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -306,8 +306,6 @@ void Lowering::LowerBlockStore(GenTreeBlk* blkNode) { // TODO-1stClassStructs: for now we can't work with STORE_BLOCK source in register. const unsigned srcLclNum = src->AsLclVar()->GetLclNum(); - INDEBUG(const LclVarDsc* lvlVar = comp->lvaGetDesc(srcLclNum);); - assert(comp->lvaVarDoNotEnregister(srcLclNum) || lvlVar->HasGCPtr()); comp->lvaSetVarDoNotEnregister(srcLclNum DEBUGARG(Compiler::DNER_BlockOp)); } diff --git a/src/coreclr/jit/lowerxarch.cpp b/src/coreclr/jit/lowerxarch.cpp index d4a36411995490..43c0df62042365 100644 --- a/src/coreclr/jit/lowerxarch.cpp +++ b/src/coreclr/jit/lowerxarch.cpp @@ -280,8 +280,6 @@ void Lowering::LowerBlockStore(GenTreeBlk* blkNode) { // TODO-1stClassStructs: for now we can't work with STORE_BLOCK source in register. const unsigned srcLclNum = src->AsLclVar()->GetLclNum(); - INDEBUG(const LclVarDsc* lvlVar = comp->lvaGetDesc(srcLclNum);); - assert(comp->lvaVarDoNotEnregister(srcLclNum) || lvlVar->HasGCPtr()); comp->lvaSetVarDoNotEnregister(srcLclNum DEBUGARG(Compiler::DNER_BlockOp)); }