From 1682809cd283ea5bda7401ab8c22af35e5fed422 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 20 Mar 2021 12:24:23 +0300 Subject: [PATCH 1/5] use VNF_StrCns for const strings --- src/coreclr/jit/valuenum.cpp | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index bae645efe0d39..54aa843027c66 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -8138,12 +8138,21 @@ void Compiler::fgValueNumberTree(GenTree* tree) if (tree->gtFlags & GTF_IND_INVARIANT) { assert(!isVolatile); // We don't expect both volatile and invariant - tree->gtVNPair = - ValueNumPair(vnStore->VNForMapSelect(VNK_Liberal, TYP_REF, ValueNumStore::VNForROH(), - addrNvnp.GetLiberal()), - vnStore->VNForMapSelect(VNK_Conservative, TYP_REF, ValueNumStore::VNForROH(), - addrNvnp.GetConservative())); - tree->gtVNPair = vnStore->VNPWithExc(tree->gtVNPair, addrXvnp); + + if (addr->OperIs(GT_CNS_INT) && addr->IsIconHandle(GTF_ICON_STR_HDL)) + { + tree->gtVNPair = vnStore->VNPairForFunc(tree->TypeGet(), VNF_StrCns, addrNvnp); + tree->gtVNPair = vnStore->VNPWithExc(tree->gtVNPair, addrXvnp); + } + else + { + tree->gtVNPair = + ValueNumPair(vnStore->VNForMapSelect(VNK_Liberal, TYP_REF, ValueNumStore::VNForROH(), + addrNvnp.GetLiberal()), + vnStore->VNForMapSelect(VNK_Conservative, TYP_REF, ValueNumStore::VNForROH(), + addrNvnp.GetConservative())); + tree->gtVNPair = vnStore->VNPWithExc(tree->gtVNPair, addrXvnp); + } } else if (isVolatile) { From 4d6d0f82f48f4ce5ce37859f482899ca731f4baf Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 20 Mar 2021 13:12:24 +0300 Subject: [PATCH 2/5] rename StrCns to LazyStrCns for helper --- src/coreclr/jit/valuenum.cpp | 2 +- src/coreclr/jit/valuenumfuncs.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 54aa843027c66..b3584f8298227 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -9607,7 +9607,7 @@ VNFunc Compiler::fgValueNumberJitHelperMethodVNFunc(CorInfoHelpFunc helpFunc) break; case CORINFO_HELP_STRCNS: - vnf = VNF_StrCns; + vnf = VNF_LazyStrCns; break; case CORINFO_HELP_CHKCASTCLASS: diff --git a/src/coreclr/jit/valuenumfuncs.h b/src/coreclr/jit/valuenumfuncs.h index 1d3906d92ddec..d804ac13d4db1 100644 --- a/src/coreclr/jit/valuenumfuncs.h +++ b/src/coreclr/jit/valuenumfuncs.h @@ -143,7 +143,8 @@ ValueNumFuncDef(JitReadyToRunNewArr, 3, false, true, false) ValueNumFuncDef(Box, 3, false, false, false) ValueNumFuncDef(BoxNullable, 3, false, false, false) -ValueNumFuncDef(StrCns, 2, false, true, false) +ValueNumFuncDef(LazyStrCns, 2, false, true, false) // lazy-initialized string literal (helper) +ValueNumFuncDef(StrCns, 2, false, true, false) // indirect for a string literal ValueNumFuncDef(Unbox, 2, false, true, false) ValueNumFuncDef(LT_UN, 2, false, false, false) // unsigned or unordered comparisons From 22812e737fda5c97404f9d10984d5cf1cd096e12 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 20 Mar 2021 13:14:54 +0300 Subject: [PATCH 3/5] clean up --- src/coreclr/jit/valuenum.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index b3584f8298227..554aae986a5f3 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -8139,7 +8139,8 @@ void Compiler::fgValueNumberTree(GenTree* tree) { assert(!isVolatile); // We don't expect both volatile and invariant - if (addr->OperIs(GT_CNS_INT) && addr->IsIconHandle(GTF_ICON_STR_HDL)) + // Is it a string literal? (it's always non-null) + if (addr->IsCnsIntOrI() && addr->IsIconHandle(GTF_ICON_STR_HDL)) { tree->gtVNPair = vnStore->VNPairForFunc(tree->TypeGet(), VNF_StrCns, addrNvnp); tree->gtVNPair = vnStore->VNPWithExc(tree->gtVNPair, addrXvnp); From bc01773631a5f6a2c4ffe86597c8ca99761aadb7 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 21 Mar 2021 21:59:26 +0300 Subject: [PATCH 4/5] Address feedback --- src/coreclr/jit/valuenum.cpp | 2 +- src/coreclr/jit/valuenumfuncs.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 554aae986a5f3..1d29ee080aa30 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -8143,7 +8143,7 @@ void Compiler::fgValueNumberTree(GenTree* tree) if (addr->IsCnsIntOrI() && addr->IsIconHandle(GTF_ICON_STR_HDL)) { tree->gtVNPair = vnStore->VNPairForFunc(tree->TypeGet(), VNF_StrCns, addrNvnp); - tree->gtVNPair = vnStore->VNPWithExc(tree->gtVNPair, addrXvnp); + assert(addrXvnp == vnStore->VNForEmptyExcSet()); } else { diff --git a/src/coreclr/jit/valuenumfuncs.h b/src/coreclr/jit/valuenumfuncs.h index d804ac13d4db1..9297115c55752 100644 --- a/src/coreclr/jit/valuenumfuncs.h +++ b/src/coreclr/jit/valuenumfuncs.h @@ -144,7 +144,7 @@ ValueNumFuncDef(Box, 3, false, false, false) ValueNumFuncDef(BoxNullable, 3, false, false, false) ValueNumFuncDef(LazyStrCns, 2, false, true, false) // lazy-initialized string literal (helper) -ValueNumFuncDef(StrCns, 2, false, true, false) // indirect for a string literal +ValueNumFuncDef(StrCns, 1, false, true, false) // indirect for a string literal ValueNumFuncDef(Unbox, 2, false, true, false) ValueNumFuncDef(LT_UN, 2, false, false, false) // unsigned or unordered comparisons From 174e4f274ffd50722c61f21765a2b4c47780a09e Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 21 Mar 2021 23:24:55 +0300 Subject: [PATCH 5/5] fix errors --- src/coreclr/jit/valuenum.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 1d29ee080aa30..b530c068b6c36 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -8143,7 +8143,7 @@ void Compiler::fgValueNumberTree(GenTree* tree) if (addr->IsCnsIntOrI() && addr->IsIconHandle(GTF_ICON_STR_HDL)) { tree->gtVNPair = vnStore->VNPairForFunc(tree->TypeGet(), VNF_StrCns, addrNvnp); - assert(addrXvnp == vnStore->VNForEmptyExcSet()); + assert(addrXvnp.BothEqual() && (addrXvnp.GetLiberal() == ValueNumStore::VNForEmptyExcSet())); } else {