Use BBF_IMPORTER for new blocks in helperexpansion.cpp #116359

EgorBo · 2025-06-06T02:15:12Z

Both "Expand runtime lookup" and "Expand static cctors" (NativeAOT) may run in Tier0/MinOpts (Debug). Convert BBF_INTERNAL to BBF_IMPORTER for new blocks (as was suggested by @eterekhin)

dotnet-policy-service · 2025-06-06T02:16:07Z

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Copilot

Pull Request Overview

This PR updates the flag conversion logic in helperexpansion.cpp to use BBF_IMPORTED for new blocks, addressing the issue raised in #116143.

Removed an unused variable (sigNode) in fgExpandRuntimeLookupsForCall.
Updated BBF_INTERNAL flag conversion to BBF_IMPORTED in both runtime lookup and static initializer expansion paths.

src/coreclr/jit/helperexpansion.cpp

EgorBo · 2025-06-06T12:14:33Z

PTAL @jakobbotsch @noahfalk

I only touched the phases that may introduce internal blocks for Tier0/MinOpts - there are a few more places for optimized code, do we need to handle them as well?

eterekhin · 2025-06-06T12:40:30Z

@EgorBo, Thank you for working on that!

I checked with this commit, unfortunately ICorDebugILFrame::GetIP still returns NO_MAPPING result on my code snippet and NO_MAP entry is there.

Attached a piece of ConfigureFormattingUiTestBase2<>.Test10 JIT dump

*************** In genIPmappingGen()
IP mapping count : 9
IL offs PROLOG : 0x00000000 ( STACK_EMPTY )
IL offs NO_MAP : 0x0000001C ( STACK_EMPTY )
IL offs 0x0000 : 0x0000002A ( STACK_EMPTY )
IL offs 0x0001 : 0x0000002B ( STACK_EMPTY )
IL offs NO_MAP : 0x00000055 ( STACK_EMPTY ) // <---
IL offs 0x000B : 0x00000079 ( CALL_INSTRUCTION )
IL offs 0x0010 : 0x00000082
IL offs 0x0011 : 0x00000083 ( STACK_EMPTY )
IL offs EPILOG : 0x00000084 ( STACK_EMPTY )

jakobbotsch · 2025-06-06T13:06:30Z

src/coreclr/jit/helperexpansion.cpp

+    if (prevBb->HasFlag(BBF_IMPORTED))
+    {
+        nullcheckBb->RemoveFlags(BBF_INTERNAL);
+        nullcheckBb->SetFlags(BBF_IMPORTED);
+
+        fastPathBb->RemoveFlags(BBF_INTERNAL);
+        fastPathBb->SetFlags(BBF_IMPORTED);
+
+        if (needsSizeCheck)
+        {
+            sizeCheckBb->RemoveFlags(BBF_INTERNAL);
+            sizeCheckBb->SetFlags(BBF_IMPORTED);
+        }
+
+        assert(block->HasFlag(BBF_IMPORTED));
+    }


The backend uses condition BBF_INTERNAL to insert the NO_MAPPING entries, and that's also the condition that QMARK expansion uses. Should it rather match that? (I.e. if !BBF_INTERNAL then also remove it from the added blocks, and add BBF_IMPORTED in that case)

Ah, indeed, thanks! Changed to match the QMARK logic

@eterekhin since you apparently have a repro set up, could you please check the latest fixe? 🙂

@EgorBo, Sure! Unfortunately, still same. Could it be because BB08 is still marked internal?

*************** Finishing PHASE Expand runtime lookups Trees after Expand runtime lookups --------------------------------------------------------------------------------------------------------------------------------------------------------------------- BBnum BBid ref try hnd preds weight [IL range] [jump] [EH region] [flags] --------------------------------------------------------------------------------------------------------------------------------------------------------------------- BB02 [0001] 1 1 [???..???)-> BB04(1) (always) i internal hascall BB04 [0003] 1 BB02 1 [???..???)-> BB03(0.5),BB05(0.5) ( cond ) internal BB05 [0004] 1 BB04 0.50 [???..???)-> BB03(1) (always) internal BB03 [0002] 2 BB04,BB05 1 [???..???)-> BB01(1) (always) i internal hascall BB01 [0000] 1 BB03 1 [000..010)-> BB07(1) (always) i hascall gcsafe BB07 [0006] 1 BB01 1 [010..010)-> BB08(0.2),BB09(0.8) ( cond ) i BB09 [0008] 1 BB07 0.80 [010..010)-> BB06(1) (always) i BB08 [0007] 1 BB07 0.20 [010..010)-> BB06(1) (always) internal BB06 [0005] 2 BB08,BB09 1 [010..012) (return) i hascall gcsafe --------------------------------------------------------------------------------------------------------------------------------------------------------------------- ------------ BB02 [0001] [???..???) -> BB04(1) (always), preds={} succs={BB04} ------------ BB04 [0003] [???..???) -> BB03(0.5),BB05(0.5) (cond), preds={BB02} succs={BB05,BB03} ***** BB04 [0003] STMT00007 ( ??? ... ??? ) N005 (???,???) [000027] ----G------ * JTRUE void N004 (???,???) [000020] J---G+-N--- \--* EQ int N002 (???,???) [000018] n---G+----- +--* IND int N001 (???,???) [000017] H----+----- | \--* CNS_INT(h) long 0x7ffade40f038 global ptr N003 (???,???) [000019] -----+----- \--* CNS_INT int 0 ------------ BB05 [0004] [???..???) -> BB03(1) (always), preds={BB04} succs={BB03} ***** BB05 [0004] STMT00008 ( ??? ... ??? ) N001 (???,???) [000021] --C-G+?---- * CALL help void CORINFO_HELP_DBG_IS_JUST_MY_CODE ------------ BB03 [0002] [???..???) -> BB01(1) (always), preds={BB04,BB05} succs={BB01} ------------ BB01 [0000] [000..010) -> BB07(1) (always), preds={BB03} succs={BB07} ***** BB01 [0000] STMT00000 ( 0x000[E-] ... 0x000 ) N001 (???,???) [000000] -----+----- * NO_OP void ***** BB01 [0000] STMT00001 ( 0x001[E-] ... 0x005 ) N002 (???,???) [000004] DA---+----- * STORE_LCL_VAR struct<System.Nullable`1, 8> V01 loc0 N001 (???,???) [000003] -----+----- \--* CNS_INT int 0 ------------ BB07 [0006] [010..010) -> BB08(0.2),BB09(0.8) (cond), preds={BB01} succs={BB09,BB08} ***** BB07 [0006] STMT00009 ( ??? ... ??? ) N014 (???,???) [000050] -A--G------ * JTRUE void N013 (???,???) [000049] JA--G------ \--* EQ int N011 (???,???) [000046] -A--G------ +--* COMMA long N009 (???,???) [000044] DA--G------ | +--* STORE_LCL_VAR long V05 tmp3 N008 (???,???) [000043] n---G------ | | \--* IND long N007 (???,???) [000036] ----------- | | \--* ADD long N005 (???,???) [000037] #---------- | | +--* IND long N004 (???,???) [000038] #---------- | | | \--* IND long N003 (???,???) [000039] ----------- | | | \--* ADD long N001 (???,???) [000040] !---------- | | | +--* LCL_VAR long V00 TypeCtx N002 (???,???) [000041] ----------- | | | \--* CNS_INT long 56 N006 (???,???) [000042] ----------- | | \--* CNS_INT long 16 N010 (???,???) [000045] ----------- | \--* LCL_VAR long V05 tmp3 N012 (???,???) [000048] ----------- \--* CNS_INT long 0 ------------ BB09 [0008] [010..010) -> BB06(1) (always), preds={BB07} succs={BB06} ***** BB09 [0008] STMT00011 ( ??? ... ??? ) N002 (???,???) [000052] DA--------- * STORE_LCL_VAR long V03 tmp1 N001 (???,???) [000047] ----------- \--* LCL_VAR long V05 tmp3 ------------ BB08 [0007] [010..010) -> BB06(1) (always), preds={BB07} succs={BB06} ***** BB08 [0007] STMT00010 ( ??? ... ??? ) N004 (???,???) [000051] DAC-G------ * STORE_LCL_VAR long V03 tmp1 N003 (???,???) [000009] --C-G+----- \--* CALL help long CORINFO_HELP_RUNTIMEHANDLE_CLASS N001 (???,???) [000007] !----+----- arg0 rcx +--* LCL_VAR long V00 TypeCtx N002 (???,???) [000008] H----+-N--- arg1 rdx \--* CNS_INT(h) long 0x7ffade4477a0 global ptr ------------ BB06 [0005] [010..012) (return), preds={BB08,BB09} succs={} ***** BB06 [0005] STMT00003 ( ??? ... ??? ) N005 (???,???) [000013] DACXG+----- * STORE_LCL_VAR ref V04 tmp2 N004 (???,???) [000006] --CXG+----- \--* CALL ref ConfigureFormattingUiTestBase2`1[System.__Canon]:ConstructViewModel2[System.__Canon](int,System.Nullable`1[int]):System.Object N001 (???,???) [000011] -----+----- gctx rcx +--* LCL_VAR long V03 tmp1 N002 (???,???) [000026] -----+----- arg2 r8 +--* LCL_FLD long V01 loc0 [+0] N003 (???,???) [000001] -----+----- arg1 rdx \--* CNS_INT int 1 ***** BB06 [0005] STMT00004 ( 0x010[--] ... ??? ) N001 (???,???) [000015] -----+----- * NOP void ***** BB06 [0005] STMT00005 ( 0x011[E-] ... 0x011 ) N001 (???,???) [000016] -----+----- * RETURN void

IL to native map looks the same as the one I attached above
In case you would like to reproduce it locally, it is quite easy, just running the code with DOTNET_JitDump=Test10
I will be happy to check the debugger call (hope once NO_MAP is gone the call will work fine)

noahfalk · 2025-06-06T20:01:31Z

there are a few more places for optimized code, do we need to handle them as well?

For optimized code the long-standing convention has been the JIT would make "best effort" to produce good mapping information in optimized code, but there is no guarantee on it. My preference would be that we fix it in optimized code too assuming it isn't particularly onerous or risky to do.

Use BBF_IMPORTER for new blocks in helperexpansion.cpp

1f95d65

dotnet-policy-service bot assigned EgorBo Jun 6, 2025

github-actions bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 6, 2025

EgorBo marked this pull request as ready for review June 6, 2025 12:13

Copilot AI review requested due to automatic review settings June 6, 2025 12:13

Copilot AI reviewed Jun 6, 2025

View reviewed changes

src/coreclr/jit/helperexpansion.cpp Show resolved Hide resolved

src/coreclr/jit/helperexpansion.cpp Outdated Show resolved Hide resolved

EgorBo force-pushed the fix-bbf-internal branch from a78ec56 to 1f95d65 Compare June 6, 2025 13:04

jakobbotsch reviewed Jun 6, 2025

View reviewed changes

Address feedback

ad68b1f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use BBF_IMPORTER for new blocks in helperexpansion.cpp #116359

Use BBF_IMPORTER for new blocks in helperexpansion.cpp #116359

EgorBo commented Jun 6, 2025 •

edited

Loading

Uh oh!

dotnet-policy-service bot commented Jun 6, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

EgorBo commented Jun 6, 2025

Uh oh!

eterekhin commented Jun 6, 2025

Uh oh!

jakobbotsch Jun 6, 2025

Uh oh!

EgorBo Jun 6, 2025

Uh oh!

eterekhin Jun 6, 2025 •

edited

Loading

Uh oh!

noahfalk commented Jun 6, 2025

Uh oh!

Uh oh!

Use BBF_IMPORTER for new blocks in helperexpansion.cpp #116359

Are you sure you want to change the base?

Use BBF_IMPORTER for new blocks in helperexpansion.cpp #116359

Conversation

EgorBo commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dotnet-policy-service bot commented Jun 6, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

EgorBo commented Jun 6, 2025

Uh oh!

eterekhin commented Jun 6, 2025

Uh oh!

jakobbotsch Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

EgorBo Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

eterekhin Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

noahfalk commented Jun 6, 2025

Uh oh!

Uh oh!

EgorBo commented Jun 6, 2025 •

edited

Loading

eterekhin Jun 6, 2025 •

edited

Loading