Skip to content

Commit 722b8a6

Browse files
authored
JIT: ensure bbflags get treated as 64 bit literals (#43451)
Expressions like `~(BBF_KEEP_BBJ_ALWAYS)` were being evaluated as 32 bit signed quantities, leading to mask value `00000000_7FFFFFFFF` instead of the desired `FFFFFFFF_7FFFFFFFF`, causing inadvertent clearing of flags with higher value. SPMI diffs showed the only flag loss that impacted codegen was `BBF_HAS_CALL`, which feeds into the CSE heuristics. So no known correctness issue, but it is certainly possible to also lose `BBF_DOMINATED_BY_EXCEPTIONAL_ENTRY` or `BBF_HAS_SUPPRESSGC_CALL` and that may be more serious.
1 parent 8dcb564 commit 722b8a6

File tree

2 files changed

+64
-57
lines changed

2 files changed

+64
-57
lines changed

src/coreclr/src/jit/block.h

Lines changed: 60 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -380,71 +380,74 @@ struct BasicBlock : private LIR::Range
380380
unsigned bbRefs; // number of blocks that can reach here, either by fall-through or a branch. If this falls to zero,
381381
// the block is unreachable.
382382

383+
#define MAKE_BBFLAG(bit) (1ULL << (bit))
384+
383385
// clang-format off
384386

385-
#define BBF_VISITED 0x00000001 // BB visited during optimizations
386-
#define BBF_MARKED 0x00000002 // BB marked during optimizations
387-
#define BBF_CHANGED 0x00000004 // input/output of this block has changed
388-
#define BBF_REMOVED 0x00000008 // BB has been removed from bb-list
389-
390-
#define BBF_DONT_REMOVE 0x00000010 // BB should not be removed during flow graph optimizations
391-
#define BBF_IMPORTED 0x00000020 // BB byte-code has been imported
392-
#define BBF_INTERNAL 0x00000040 // BB has been added by the compiler
393-
#define BBF_FAILED_VERIFICATION 0x00000080 // BB has verification exception
394-
395-
#define BBF_TRY_BEG 0x00000100 // BB starts a 'try' block
396-
#define BBF_FUNCLET_BEG 0x00000200 // BB is the beginning of a funclet
397-
#define BBF_HAS_NULLCHECK 0x00000400 // BB contains a null check
398-
399-
#define BBF_RUN_RARELY 0x00001000 // BB is rarely run (catch clauses, blocks with throws etc)
400-
#define BBF_LOOP_HEAD 0x00002000 // BB is the head of a loop
401-
#define BBF_LOOP_CALL0 0x00004000 // BB starts a loop that sometimes won't call
402-
#define BBF_LOOP_CALL1 0x00008000 // BB starts a loop that will always call
403-
404-
#define BBF_HAS_LABEL 0x00010000 // BB needs a label
405-
#define BBF_JMP_TARGET 0x00020000 // BB is a target of an implicit/explicit jump
406-
#define BBF_HAS_JMP 0x00040000 // BB executes a JMP instruction (instead of return)
407-
#define BBF_GC_SAFE_POINT 0x00080000 // BB has a GC safe point (a call). More abstractly, BB does not require a
408-
// (further) poll -- this may be because this BB has a call, or, in some
409-
// cases, because the BB occurs in a loop, and we've determined that all
410-
// paths in the loop body leading to BB include a call.
411-
412-
#define BBF_HAS_VTABREF 0x00100000 // BB contains reference of vtable
413-
#define BBF_HAS_IDX_LEN 0x00200000 // BB contains simple index or length expressions on an array local var.
414-
#define BBF_HAS_NEWARRAY 0x00400000 // BB contains 'new' of an array
415-
#define BBF_HAS_NEWOBJ 0x00800000 // BB contains 'new' of an object type.
387+
#define BBF_VISITED MAKE_BBFLAG( 0) // BB visited during optimizations
388+
#define BBF_MARKED MAKE_BBFLAG( 1) // BB marked during optimizations
389+
#define BBF_CHANGED MAKE_BBFLAG( 2) // input/output of this block has changed
390+
#define BBF_REMOVED MAKE_BBFLAG( 3) // BB has been removed from bb-list
391+
392+
#define BBF_DONT_REMOVE MAKE_BBFLAG( 4) // BB should not be removed during flow graph optimizations
393+
#define BBF_IMPORTED MAKE_BBFLAG( 5) // BB byte-code has been imported
394+
#define BBF_INTERNAL MAKE_BBFLAG( 6) // BB has been added by the compiler
395+
#define BBF_FAILED_VERIFICATION MAKE_BBFLAG( 7) // BB has verification exception
396+
397+
#define BBF_TRY_BEG MAKE_BBFLAG( 8) // BB starts a 'try' block
398+
#define BBF_FUNCLET_BEG MAKE_BBFLAG( 9) // BB is the beginning of a funclet
399+
#define BBF_HAS_NULLCHECK MAKE_BBFLAG(10) // BB contains a null check
400+
#define BBF_HAS_SUPPRESSGC_CALL MAKE_BBFLAG(11) // BB contains a call to a method with SuppressGCTransitionAttribute
401+
402+
#define BBF_RUN_RARELY MAKE_BBFLAG(12) // BB is rarely run (catch clauses, blocks with throws etc)
403+
#define BBF_LOOP_HEAD MAKE_BBFLAG(13) // BB is the head of a loop
404+
#define BBF_LOOP_CALL0 MAKE_BBFLAG(14) // BB starts a loop that sometimes won't call
405+
#define BBF_LOOP_CALL1 MAKE_BBFLAG(15) // BB starts a loop that will always call
406+
407+
#define BBF_HAS_LABEL MAKE_BBFLAG(16) // BB needs a label
408+
#define BBF_JMP_TARGET MAKE_BBFLAG(17) // BB is a target of an implicit/explicit jump
409+
#define BBF_HAS_JMP MAKE_BBFLAG(18) // BB executes a JMP instruction (instead of return)
410+
#define BBF_GC_SAFE_POINT MAKE_BBFLAG(19) // BB has a GC safe point (a call). More abstractly, BB does not require a
411+
// (further) poll -- this may be because this BB has a call, or, in some
412+
// cases, because the BB occurs in a loop, and we've determined that all
413+
// paths in the loop body leading to BB include a call.
414+
415+
#define BBF_HAS_VTABREF MAKE_BBFLAG(20) // BB contains reference of vtable
416+
#define BBF_HAS_IDX_LEN MAKE_BBFLAG(21) // BB contains simple index or length expressions on an array local var.
417+
#define BBF_HAS_NEWARRAY MAKE_BBFLAG(22) // BB contains 'new' of an array
418+
#define BBF_HAS_NEWOBJ MAKE_BBFLAG(23) // BB contains 'new' of an object type.
416419

417420
#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
418421

419-
#define BBF_FINALLY_TARGET 0x01000000 // BB is the target of a finally return: where a finally will return during
420-
// non-exceptional flow. Because the ARM calling sequence for calling a
421-
// finally explicitly sets the return address to the finally target and jumps
422-
// to the finally, instead of using a call instruction, ARM needs this to
423-
// generate correct code at the finally target, to allow for proper stack
424-
// unwind from within a non-exceptional call to a finally.
422+
#define BBF_FINALLY_TARGET MAKE_BBFLAG(24) // BB is the target of a finally return: where a finally will return during
423+
// non-exceptional flow. Because the ARM calling sequence for calling a
424+
// finally explicitly sets the return address to the finally target and jumps
425+
// to the finally, instead of using a call instruction, ARM needs this to
426+
// generate correct code at the finally target, to allow for proper stack
427+
// unwind from within a non-exceptional call to a finally.
425428

426429
#endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
427430

428-
#define BBF_BACKWARD_JUMP 0x02000000 // BB is surrounded by a backward jump/switch arc
429-
#define BBF_RETLESS_CALL 0x04000000 // BBJ_CALLFINALLY that will never return (and therefore, won't need a paired
430-
// BBJ_ALWAYS); see isBBCallAlwaysPair().
431-
#define BBF_LOOP_PREHEADER 0x08000000 // BB is a loop preheader block
432-
433-
#define BBF_COLD 0x10000000 // BB is cold
434-
#define BBF_PROF_WEIGHT 0x20000000 // BB weight is computed from profile data
435-
#define BBF_IS_LIR 0x40000000 // Set if the basic block contains LIR (as opposed to HIR)
436-
#define BBF_KEEP_BBJ_ALWAYS 0x80000000 // A special BBJ_ALWAYS block, used by EH code generation. Keep the jump kind
437-
// as BBJ_ALWAYS. Used for the paired BBJ_ALWAYS block following the
438-
// BBJ_CALLFINALLY block, as well as, on x86, the final step block out of a
439-
// finally.
440-
441-
#define BBF_CLONED_FINALLY_BEGIN 0x100000000 // First block of a cloned finally region
442-
#define BBF_CLONED_FINALLY_END 0x200000000 // Last block of a cloned finally region
443-
#define BBF_HAS_CALL 0x400000000 // BB contains a call
444-
#define BBF_DOMINATED_BY_EXCEPTIONAL_ENTRY 0x800000000 // Block is dominated by exceptional entry.
445-
#define BBF_BACKWARD_JUMP_TARGET 0x1000000000 // Block is a target of a backward jump
446-
#define BBF_PATCHPOINT 0x2000000000 // Block is a patchpoint
447-
#define BBF_HAS_SUPPRESSGC_CALL 0x4000000000 // BB contains a call to a method with SuppressGCTransitionAttribute
431+
#define BBF_BACKWARD_JUMP MAKE_BBFLAG(25) // BB is surrounded by a backward jump/switch arc
432+
#define BBF_RETLESS_CALL MAKE_BBFLAG(26) // BBJ_CALLFINALLY that will never return (and therefore, won't need a paired
433+
// BBJ_ALWAYS); see isBBCallAlwaysPair().
434+
#define BBF_LOOP_PREHEADER MAKE_BBFLAG(27) // BB is a loop preheader block
435+
436+
#define BBF_COLD MAKE_BBFLAG(28) // BB is cold
437+
#define BBF_PROF_WEIGHT MAKE_BBFLAG(29) // BB weight is computed from profile data
438+
#define BBF_IS_LIR MAKE_BBFLAG(30) // Set if the basic block contains LIR (as opposed to HIR)
439+
#define BBF_KEEP_BBJ_ALWAYS MAKE_BBFLAG(31) // A special BBJ_ALWAYS block, used by EH code generation. Keep the jump kind
440+
// as BBJ_ALWAYS. Used for the paired BBJ_ALWAYS block following the
441+
// BBJ_CALLFINALLY block, as well as, on x86, the final step block out of a
442+
// finally.
443+
444+
#define BBF_CLONED_FINALLY_BEGIN MAKE_BBFLAG(32) // First block of a cloned finally region
445+
#define BBF_CLONED_FINALLY_END MAKE_BBFLAG(33) // Last block of a cloned finally region
446+
#define BBF_HAS_CALL MAKE_BBFLAG(34) // BB contains a call
447+
#define BBF_DOMINATED_BY_EXCEPTIONAL_ENTRY MAKE_BBFLAG(35) // Block is dominated by exceptional entry.
448+
449+
#define BBF_BACKWARD_JUMP_TARGET MAKE_BBFLAG(36) // Block is a target of a backward jump
450+
#define BBF_PATCHPOINT MAKE_BBFLAG(37) // Block is a patchpoint
448451

449452
// clang-format on
450453

src/coreclr/src/jit/patchpoint.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ class PatchpointTransformer
5454
{
5555
if (block->bbFlags & BBF_PATCHPOINT)
5656
{
57+
// Clear the patchpoint flag.
58+
//
59+
block->bbFlags &= ~BBF_PATCHPOINT;
60+
5761
// If block is in a handler region, don't insert a patchpoint.
5862
// We can't OSR from funclets.
5963
//

0 commit comments

Comments
 (0)