PIX: Don't seek beyond terminator instructions (value-to-declare pass) (

microsoft#3855) (microsoft#3856)

Background: this pass is trying to find all dbg.value and replace them with dbg.declare. In the code being changed, the pass is trying to seek a valid location at which to insert the dbg.declare. It has to come after the value to which it applies (which isn't true of the dbg.value). So there's this little loop trying to move forward to find the right instruction before which to insert new stuff.

I was expecting getNextNode to return null when there is no next node. When called on a terminator, it actually returns a non-null but malformed instruction pointer. So we have to explicitly check for terminators in this loop.

This really short basic block tripped up the pass:

; <label>:274                                     ; preds = %.lr.ph55
  %RawBufferLoad = call %dx.types.ResRet.i32 @dx.op.rawBufferLoad.i32(i32 139, %dx.types.Handle %lightBuffer_texture_structbuf, i32 %lightIndex.0, i32 28, i8 1, i32 4), !dbg !384
  %275 = extractvalue %dx.types.ResRet.i32 %RawBufferLoad, 0, !dbg !384
  switch i32 %275, label %288 [
    i32 0, label %276
    i32 1, label %280
    i32 2, label %284
  ], !dbg !397
I think the pass could be smarter about seeking the right insertion point for the dbg.declare. It's currently assuming that the dbg.value always succeeds the value to which it refers, but as in this case, that's not always true. But that's a project for another day.

(cherry picked from commit 650de80)

lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp

@@ -704,40 +704,44 @@ void DxilDbgValueToDbgDeclare::handleDbgValue(
   auto* instruction = llvm::dyn_cast<llvm::Instruction>(V);
   if (instruction != nullptr) {
     instruction = instruction->getNextNode();
-    if (instruction != nullptr) {
+    if (instruction != nullptr && !llvm::isa<TerminatorInst>(instruction)) {
       // Drivers may crash if phi nodes aren't always at the top of a block,
       // so we must skip over them before inserting instructions.
       do {
         instruction = instruction->getNextNode();
-      } while (instruction != nullptr && llvm::isa<llvm::PHINode>(instruction));
+      } while (instruction != nullptr && 
+          llvm::isa<llvm::PHINode>(instruction) &&
+          !llvm::isa<TerminatorInst>(instruction));
 
-      B.SetInsertPoint(instruction);
-
-      B.SetCurrentDebugLocation(llvm::DebugLoc());
-      auto *Zero = B.getInt32(0);
-
-      // Now traverse a list of pairs {Scalar Value, InitialOffset + Offset}.
-      // InitialOffset is the offset from DbgValue's expression (i.e., the
-      // offset from the Variable's start), and Offset is the Scalar Value's
-      // packed offset from DbgValue's value.
-      for (const ValueAndOffset &VO : SplitValue(V, InitialOffset, B)) {
-
-        OffsetInBits AlignedOffset;
-        if (!Offsets.GetAlignedOffsetFromPackedOffset(VO.m_PackedOffset,
-                                                      &AlignedOffset)) {
-          continue;
-        }
-
-        auto *AllocaInst = Register->GetRegisterForAlignedOffset(AlignedOffset);
-        if (AllocaInst == nullptr) {
-          assert(!"Failed to find alloca for var[offset]");
-          continue;
-        }
-
-        if (AllocaInst->getAllocatedType()->getArrayElementType() ==
-            VO.m_V->getType()) {
-          auto *GEP = B.CreateGEP(AllocaInst, {Zero, Zero});
-          B.CreateStore(VO.m_V, GEP);
+      if(instruction != nullptr && !llvm::isa<TerminatorInst>(instruction)) {
+        B.SetInsertPoint(instruction);
+
+        B.SetCurrentDebugLocation(llvm::DebugLoc());
+        auto *Zero = B.getInt32(0);
+
+        // Now traverse a list of pairs {Scalar Value, InitialOffset + Offset}.
+        // InitialOffset is the offset from DbgValue's expression (i.e., the
+        // offset from the Variable's start), and Offset is the Scalar Value's
+        // packed offset from DbgValue's value.
+        for (const ValueAndOffset &VO : SplitValue(V, InitialOffset, B)) {
+
+          OffsetInBits AlignedOffset;
+          if (!Offsets.GetAlignedOffsetFromPackedOffset(VO.m_PackedOffset,
+                                                        &AlignedOffset)) {
+            continue;
+          }
+
+          auto *AllocaInst = Register->GetRegisterForAlignedOffset(AlignedOffset);
+          if (AllocaInst == nullptr) {
+            assert(!"Failed to find alloca for var[offset]");
+            continue;
+          }
+
+          if (AllocaInst->getAllocatedType()->getArrayElementType() ==
+              VO.m_V->getType()) {
+            auto *GEP = B.CreateGEP(AllocaInst, {Zero, Zero});
+            B.CreateStore(VO.m_V, GEP);
+          }
         }
       }
     }