From bc3b9ef981534f8f121d487ba8512ec6b21c5c6c Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 1 Feb 2023 12:24:29 +0100 Subject: [PATCH 1/3] Don't clone a large tree in impRuntimeLookupToTree --- src/coreclr/jit/importer.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index c691c64f398a93..1c8f0dddda084c 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -1893,9 +1893,11 @@ GenTree* Compiler::impRuntimeLookupToTree(CORINFO_RESOLVED_TOKEN* pResolvedToken GenTreeCall* helperCall = gtNewHelperCallNode(pRuntimeLookup->helper, TYP_I_IMPL, ctxTree, argNode); + GenTree* handleForResult = fgInsertCommaFormTemp(&handleForNullCheck); + lvaTable[handleForResult->AsLclVarCommon()->GetLclNum()].lvIsCSE = true; + // Check for null and possibly call helper - GenTree* nullCheck = gtNewOperNode(GT_NE, TYP_INT, handleForNullCheck, gtNewIconNode(0, TYP_I_IMPL)); - GenTree* handleForResult = gtCloneExpr(handleForNullCheck); + GenTree* nullCheck = gtNewOperNode(GT_NE, TYP_INT, handleForNullCheck, gtNewIconNode(0, TYP_I_IMPL)); GenTree* result = nullptr; From 1ba2515851817100a3b3d666b10bfc66c02f22d1 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 6 Feb 2023 03:28:03 +0100 Subject: [PATCH 2/3] Update importer.cpp --- src/coreclr/jit/importer.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 663d053c75b580..cc0bce0529036e 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -1889,12 +1889,10 @@ GenTree* Compiler::impRuntimeLookupToTree(CORINFO_RESOLVED_TOKEN* pResolvedToken // Call the helper // - Setup argNode with the pointer to the signature returned by the lookup - GenTree* argNode = gtNewIconEmbHndNode(pRuntimeLookup->signature, nullptr, GTF_ICON_GLOBAL_PTR, compileTimeHandle); - - GenTreeCall* helperCall = gtNewHelperCallNode(pRuntimeLookup->helper, TYP_I_IMPL, ctxTree, argNode); - - GenTree* handleForResult = fgInsertCommaFormTemp(&handleForNullCheck); - lvaTable[handleForResult->AsLclVarCommon()->GetLclNum()].lvIsCSE = true; + GenTree* argNode = + gtNewIconEmbHndNode(pRuntimeLookup->signature, nullptr, GTF_ICON_GLOBAL_PTR, compileTimeHandle); + GenTreeCall* helperCall = gtNewHelperCallNode(pRuntimeLookup->helper, TYP_I_IMPL, ctxTree, argNode); + GenTree* handleForResult = fgInsertCommaFormTemp(&handleForNullCheck); // Check for null and possibly call helper GenTree* nullCheck = gtNewOperNode(GT_NE, TYP_INT, handleForNullCheck, gtNewIconNode(0, TYP_I_IMPL)); From 22b61ad87140410e17e3ccb51d5659872c902e4f Mon Sep 17 00:00:00 2001 From: EgorBo Date: Mon, 6 Feb 2023 09:46:47 +0100 Subject: [PATCH 3/3] minimize regressions --- src/coreclr/jit/importer.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index cc0bce0529036e..58b0ba68e29853 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -1889,10 +1889,13 @@ GenTree* Compiler::impRuntimeLookupToTree(CORINFO_RESOLVED_TOKEN* pResolvedToken // Call the helper // - Setup argNode with the pointer to the signature returned by the lookup - GenTree* argNode = - gtNewIconEmbHndNode(pRuntimeLookup->signature, nullptr, GTF_ICON_GLOBAL_PTR, compileTimeHandle); - GenTreeCall* helperCall = gtNewHelperCallNode(pRuntimeLookup->helper, TYP_I_IMPL, ctxTree, argNode); - GenTree* handleForResult = fgInsertCommaFormTemp(&handleForNullCheck); + GenTree* argNode = gtNewIconEmbHndNode(pRuntimeLookup->signature, nullptr, GTF_ICON_GLOBAL_PTR, compileTimeHandle); + GenTreeCall* helperCall = gtNewHelperCallNode(pRuntimeLookup->helper, TYP_I_IMPL, ctxTree, argNode); + + // handleForNullCheck is used for both nullcheck and as a return value. The tree is a bit verbose so we'd better + // save it to a local to make IR smaller. Although, the local leads to undesired size regression in MinOpts + GenTree* handleForResult = + opts.OptimizationEnabled() ? fgInsertCommaFormTemp(&handleForNullCheck) : gtCloneExpr(handleForNullCheck); // Check for null and possibly call helper GenTree* nullCheck = gtNewOperNode(GT_NE, TYP_INT, handleForNullCheck, gtNewIconNode(0, TYP_I_IMPL));