PIX: Change insertion point to after referenced value (#3746)

The value-to-declare pass replaces dbg.value with dbg.declare and some getElementPtr/store pairs that the PIX instrumentation uses to discover values. The problem here was that the dbg.value could reside in the program prior to the value it was referring to, and this seems to be legal. It's not correct once the dbg.value has been replaced with the GEP/store pairs, since the store was trying to operate on a value that had not yet been created. The solution is to move the builder's insertion point to that of the value itself, rather than that of the dbg.value.

Also in this change: a simple pair of copy-paste typos in the OffsetManager class.

lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp

@@ -146,13 +146,13 @@ public:
         AlignedOffset);
   }
 
-  OffsetInBits GetPackedOffsetFromAlignedOffset(
+  bool GetPackedOffsetFromAlignedOffset(
       OffsetInBits AlignedOffset,
       OffsetInBits *PackedOffset
   ) const
   {
     return GetOffsetWithMap(
-        m_PackedOffsetToAlignedOffset,
+        m_AlignedOffsetToPackedOffset,
         AlignedOffset,
         PackedOffset);
   }
@@ -417,34 +417,39 @@ void DxilDbgValueToDbgDeclare::handleDbgValue(
 
   const OffsetInBits InitialOffset = PackedOffsetFromVar;
   llvm::IRBuilder<> B(DbgValue->getCalledFunction()->getContext());
-  B.SetInsertPoint(DbgValue);
-  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* instruction = llvm::dyn_cast<llvm::Instruction>(V);
+  if (instruction != nullptr) {
+    instruction = instruction->getNextNode();
+    if (instruction != nullptr) {
+      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;
-    }
+        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 (AllocaInst->getAllocatedType()->getArrayElementType() ==
+            VO.m_V->getType()) {
+          auto *GEP = B.CreateGEP(AllocaInst, {Zero, Zero});
+          B.CreateStore(VO.m_V, GEP);
+        }
+      }
     }
   }
 }

tools/clang/unittests/HLSL/PixTest.cpp

@@ -1887,12 +1887,17 @@ void main()
 
     auto Testables = TestStructAnnotationCase(hlsl, optimization);
 
+    // 2 in unoptimized case (one for each instance of smallPayload)
+    // 1 in optimized case (cuz p2 aliases over p)
     VERIFY_IS_TRUE(Testables.OffsetAndSizes.size() >= 1);
-    VERIFY_ARE_EQUAL(1, Testables.OffsetAndSizes[0].countOfMembers);
-    VERIFY_ARE_EQUAL(0, Testables.OffsetAndSizes[0].offset);
-    VERIFY_ARE_EQUAL(32, Testables.OffsetAndSizes[0].size);
 
-    VERIFY_ARE_EQUAL(2, Testables.AllocaWrites.size());
+    for (const auto& os : Testables.OffsetAndSizes) {
+      VERIFY_ARE_EQUAL(1, os.countOfMembers);
+      VERIFY_ARE_EQUAL(0, os.offset);
+      VERIFY_ARE_EQUAL(32, os.size);
+    }
+
+    VERIFY_ARE_EQUAL(1, Testables.AllocaWrites.size());
   }
 }