-
Notifications
You must be signed in to change notification settings - Fork 5.5k
constant fold string.concat #127655
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
constant fold string.concat #127655
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3599,6 +3599,60 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd, | |||||
| break; | ||||||
| } | ||||||
|
|
||||||
| case NI_System_String_Concat: | ||||||
| { | ||||||
| // Recognize the static String.Concat(string, string [, string [, string]]) | ||||||
| // overloads. We mark the call as "special" so morph and VN can later | ||||||
| // attempt to constant-fold it into a single frozen string handle. | ||||||
| if (((methodFlags & CORINFO_FLG_STATIC) == 0) || (sig->numArgs < 2) || (sig->numArgs > 4) || | ||||||
| (sig->retType != CORINFO_TYPE_CLASS)) | ||||||
| { | ||||||
| break; | ||||||
| } | ||||||
|
|
||||||
| // All arguments must be System.String, matching the String.Concat | ||||||
| // string-only overloads. Bail otherwise (e.g. params object[]). | ||||||
| bool allStringArgs = true; | ||||||
| CORINFO_ARG_LIST_HANDLE sigArg = sig->args; | ||||||
| for (unsigned i = 0; i < sig->numArgs; i++) | ||||||
| { | ||||||
| CORINFO_CLASS_HANDLE argClass = NO_CLASS_HANDLE; | ||||||
| CorInfoType argType = strip(info.compCompHnd->getArgType(sig, sigArg, &argClass)); | ||||||
| if (argType != CORINFO_TYPE_CLASS) | ||||||
| { | ||||||
| allStringArgs = false; | ||||||
| break; | ||||||
| } | ||||||
| // For CORINFO_TYPE_CLASS args, getArgType may return a null | ||||||
| // class handle; query it explicitly and require it to be the | ||||||
| // owning System.String class. | ||||||
| if (argClass == NO_CLASS_HANDLE) | ||||||
| { | ||||||
| argClass = info.compCompHnd->getArgClass(sig, sigArg); | ||||||
| } | ||||||
| if (argClass != clsHnd) | ||||||
| { | ||||||
| allStringArgs = false; | ||||||
| break; | ||||||
| } | ||||||
| sigArg = info.compCompHnd->getArgNext(sigArg); | ||||||
| } | ||||||
|
|
||||||
| if (!allStringArgs) | ||||||
| { | ||||||
| break; | ||||||
| } | ||||||
|
|
||||||
| // Mark all string-only Concat overloads as special so both | ||||||
| // morph (gtFoldExprCall) and VN-based folding | ||||||
| // (optVNBasedFoldExpr_Call_StringConcat) can later try to fold | ||||||
| // them. The actual inline-suppression decision in | ||||||
| // impMarkInlineCandidateHelper additionally requires every | ||||||
| // argument to currently be a GT_CNS_STR. | ||||||
| isSpecial = true; | ||||||
| break; | ||||||
| } | ||||||
|
|
||||||
| case NI_System_String_get_Chars: | ||||||
| { | ||||||
| GenTree* op2 = impPopStack().val; | ||||||
|
|
@@ -8253,6 +8307,34 @@ void Compiler::impMarkInlineCandidateHelper(GenTreeCall* call, | |||||
| return; | ||||||
| } | ||||||
|
|
||||||
| // Don't inline String.Concat when all arguments are constant strings. | ||||||
| // We let the call survive so morph and VN-based folding can replace it | ||||||
| // with a single frozen string handle. Inlining the call here would defeat | ||||||
| // that fold. We deliberately allow inlining when at least one argument is | ||||||
| // not a constant string — VN-based folding handles a few extra cases, but | ||||||
| // most non-constant calls are still better off inlined. | ||||||
| if (call->IsSpecialIntrinsic() && (lookupNamedIntrinsic(call->gtCallMethHnd) == NI_System_String_Concat)) | ||||||
| { | ||||||
| bool allCns = true; | ||||||
| for (CallArg& arg : call->gtArgs.Args()) | ||||||
| { | ||||||
| if (arg.GetWellKnownArg() != WellKnownArg::None) | ||||||
| { | ||||||
| continue; | ||||||
| } | ||||||
| if (!arg.GetNode()->OperIs(GT_CNS_STR)) | ||||||
| { | ||||||
| allCns = false; | ||||||
| break; | ||||||
| } | ||||||
| } | ||||||
| if (allCns) | ||||||
| { | ||||||
| inlineResult->NoteFatal(InlineObservation::CALLEE_IS_FOLDABLE_INTRINSIC); | ||||||
|
||||||
| inlineResult->NoteFatal(InlineObservation::CALLEE_IS_FOLDABLE_INTRINSIC); | |
| inlineResult->NoteFatal(InlineObservation::CALLSITE_IS_FOLDABLE_INTRINSIC); |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -43,6 +43,7 @@ INLINE_OBSERVATION(IS_JIT_NOINLINE, bool, "noinline per JitNoinline" | |||||
| INLINE_OBSERVATION(IS_NOINLINE, bool, "noinline per IL/cached result", FATAL, CALLEE) | ||||||
| INLINE_OBSERVATION(IS_SYNCHRONIZED, bool, "is synchronized", FATAL, CALLEE) | ||||||
| INLINE_OBSERVATION(IS_VM_NOINLINE, bool, "noinline per VM", FATAL, CALLEE) | ||||||
| INLINE_OBSERVATION(IS_FOLDABLE_INTRINSIC, bool, "foldable intrinsic - keep as call", FATAL, CALLEE) | ||||||
|
||||||
| INLINE_OBSERVATION(IS_FOLDABLE_INTRINSIC, bool, "foldable intrinsic - keep as call", FATAL, CALLEE) | |
| INLINE_OBSERVATION(IS_FOLDABLE_INTRINSIC, bool, "foldable intrinsic - keep as call", FATAL, CALLSITE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says the importer marking the call as a special intrinsic "requires every argument to be a GT_CNS_STR", but
impIntrinsicmarks the string-only overloads as special regardless of whether the args are constant; the folding logic below is what requiresGT_CNS_STR. Consider adjusting the comment to avoid implying stronger invariants than actually enforced.