fgOptimizeBranch loses bbFlags while duplicating the block dotnet/coreclr#6886

1. When `fgOptimizeBranch` procedure duplicates the conditional basic block it does
not copy flags of the `bDest` block to the `bJump` block. For example, if a flag
`BBF_HAS_NEWOBJ` was set in `bDest`, but not in `bJump` when the branch optimization
is done the `bbFlags` of `bJump` will not reflect the fact that we have new object
construction in this block.

2. In `DEBUG` always check if `bbFlags` has `BBF_HAS_NEWOBJ` flag set if at least one
`GT_ALLOCOBJ` canonical node found. This helps to identify situations when compiler loses `BBF_HAS_NEWOBJ` flag.

Commit migrated from https://github.com/dotnet/coreclr/commit/12f327477b6c1e18b6db7d0b2ec3445ef3997c17

diff --git a/src/coreclr/src/jit/flowgraph.cpp b/src/coreclr/src/jit/flowgraph.cpp
index a1d4aaf..ced582b 100644 (file)
--- a/src/coreclr/src/jit/flowgraph.cpp
+++ b/src/coreclr/src/jit/flowgraph.cpp
@@ -14170,6 +14170,10 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump)
     /* Mark the jump dest block as being a jump target */
     bJump->bbJumpDest->bbFlags |= BBF_JMP_TARGET | BBF_HAS_LABEL;
 
+    // We need to update the following flags of the bJump block if they were set in the bbJumpDest block
+    bJump->bbFlags |= (bJump->bbJumpDest->bbFlags
+                    & (BBF_HAS_NEWOBJ | BBF_HAS_NEWARRAY | BBF_HAS_NULLCHECK | BBF_HAS_IDX_LEN | BBF_HAS_VTABREF));
+
     /* Update bbRefs and bbPreds */
 
     // bJump now falls through into the next block

diff --git a/src/coreclr/src/jit/objectalloc.cpp b/src/coreclr/src/jit/objectalloc.cpp
index 1921e20..2e19f43 100644 (file)
--- a/src/coreclr/src/jit/objectalloc.cpp
+++ b/src/coreclr/src/jit/objectalloc.cpp
@@ -66,10 +66,13 @@ void ObjectAllocator::MorphAllocObjNodes()
 
     foreach_block(comp, block)
     {
-        if ((block->bbFlags & BBF_HAS_NEWOBJ) == 0)
+        const bool basicBlockHasNewObj = (block->bbFlags & BBF_HAS_NEWOBJ) == BBF_HAS_NEWOBJ;
+#ifndef DEBUG
+        if (!basicBlockHasNewObj)
         {
             continue;
         }
+#endif // DEBUG
 
         for (GenTreeStmt* stmt = block->firstStmt(); stmt; stmt = stmt->gtNextStmt)
         {
@@ -90,6 +93,7 @@ void ObjectAllocator::MorphAllocObjNodes()
 
             if (canonicalAllocObjFound)
             {
+                assert(basicBlockHasNewObj);
                 //------------------------------------------------------------------------
                 // We expect the following expression tree at this point
                 //  *  GT_STMT   void  (top level)