Don't replace Undef with null value for Constants

[ykhatav]

Summary

The following patch - (https://reviews.llvm.org/D117300) introduced null's into the debug information and those nulls are expected to be undef values. The RAUW debug info handlers are for instructions, not constants and the debug information expects the value to be undef and not null. This patch tries to fix this behavior by not replacing undef with null value in Constants for debug info metadata.

[bwyma]

In a similar fashion to how instructions are handled according to Delete an IR-level instruction, when a constant is destroyed and cannot be RAUW'd, updating the debug information uses of the constant to undef seems like the correct approach. LGTM.

[StephenTozer]

Good fix, LGTM.

llvm/test/DebugInfo/X86/undef-type-md.ll

21

Nit, can this attribute be removed?

[ykhatav]

Removed the attribute

[fhahn]

Looks like this was submitted as e83543f8c2ef

@ykhatav it looks like it wasn't closed automatically because the commit message was all one line. Please make sure the commit message is properly structured in the future.

llvm/include/llvm/IR/Metadata.h

public:
   }
 
   LLVMContext &getContext() const { return Context; }
 
   /// Replace all uses of this with MD.
   ///
   /// Replace all uses of this with \c MD, which is allowed to be null.
   void replaceAllUsesWith(Metadata *MD);
-
+  /// Replace all uses of the constant with Undef in debug info metadata
+  static void SalvageDebugInfo(const Constant &C);
   /// Returns the list of all DIArgList users of this.
   SmallVector<Metadata *> getAllArgListUsers();
 
   /// Resolve all uses of this.
   ///
   /// Resolve all uses of this, turning off RAUW permanently.  If \c
   /// ResolveUsers, call \a MDNode::resolve() on any users whose last operand
   /// is resolved.

llvm/lib/IR/Constants.cpp

while (I != E) {
     // Just removed User, so the iterator was invalidated.
     // Since we return immediately upon finding a live user, we can always
     // restart from user_begin().
     if (RemoveDeadUsers)
       I = C->user_begin();
     else
       ++I;
   }
-
-  if (RemoveDeadUsers)
+  if (RemoveDeadUsers) {
+    // If C is only used by metadata, it should not be preserved but should
+    // have its uses replaced.
+    ReplaceableMetadataImpl::SalvageDebugInfo(*C);
     const_cast<Constant *>(C)->destroyConstant();
+  }
 
   return true;
 }
 
 void Constant::removeDeadConstantUsers() const {
   Value::const_user_iterator I = user_begin(), E = user_end();
   Value::const_user_iterator LastNonDeadUser = E;
   while (I != E) {

llvm/lib/IR/Metadata.cpp

void ReplaceableMetadataImpl::moveRef(void *Ref, void *New,
   // Check that the references are direct if there's no owner.
   (void)MD;
   assert((OwnerAndIndex.first || *static_cast<Metadata **>(Ref) == &MD) &&
          "Reference without owner must be direct");
   assert((OwnerAndIndex.first || *static_cast<Metadata **>(New) == &MD) &&
          "Reference without owner must be direct");
 }
 
+void ReplaceableMetadataImpl::SalvageDebugInfo(const Constant &C) {
+  if (!C.isUsedByMetadata()) {
+    return;
+  }
+
+  LLVMContext &Context = C.getType()->getContext();
+  auto &Store = Context.pImpl->ValuesAsMetadata;
+  auto I = Store.find(&C);
+  ValueAsMetadata *MD = I->second;
+  using UseTy =
+      std::pair<void *, std::pair<MetadataTracking::OwnerTy, uint64_t>>;
+  // Copy out uses and update value of Constant used by debug info metadata with undef below
+  SmallVector<UseTy, 8> Uses(MD->UseMap.begin(), MD->UseMap.end());
+
+  for (const auto &Pair : Uses) {
+    MetadataTracking::OwnerTy Owner = Pair.second.first;
+    if (!Owner)
+      continue;
+    if (!Owner.is<Metadata *>())
+      continue;
+    auto *OwnerMD = dyn_cast<MDNode>(Owner.get<Metadata *>());
+    if (!OwnerMD)
+      continue;
+    if (isa<DINode>(OwnerMD)) {
+      OwnerMD->handleChangedOperand(
+          Pair.first, ValueAsMetadata::get(UndefValue::get(C.getType())));
+    }
+  }
+}
+
 void ReplaceableMetadataImpl::replaceAllUsesWith(Metadata *MD) {
   if (UseMap.empty())
     return;
 
   // Copy out uses since UseMap will get touched below.
   using UseTy = std::pair<void *, std::pair<OwnerTy, uint64_t>>;
   SmallVector<UseTy, 8> Uses(UseMap.begin(), UseMap.end());
   llvm::sort(Uses, [](const UseTy &L, const UseTy &R) {

llvm/test/DebugInfo/X86/undef-type-md.ll

+; RUN:  opt -S -ipsccp %S/undef-type-md.ll | FileCheck %s
+; CHECK: llvm.nondebug.metadata = !{[[NONDEBUG_METADATA:![0-9]+]]}
+; CHECK: [[NONDEBUG_METADATA]] =  distinct !{null} 
+; CHECK: !DITemplateValueParameter({{.*}} value: %class.1 addrspace(1)* undef)
+
+; ModuleID = '<stdin>'
+source_filename = "test.cpp"
+
+%"struct.1" = type <{ float, i32, i8, [3 x i8] }>
+%"class.1" = type { %"struct.1" }
+
+@extern_const = external addrspace(1) constant { { float, i32, i8 } }
+
+; Function Attrs: convergent mustprogress norecurse
+define linkonce_odr spir_func void @foo() #0 align 2 !dbg !5 {
+entry:
+  %0 = alloca %"class.1", align 8
+  ret void
+}
+
+attributes #0 = { nofree nosync nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4}
+!llvm.nondebug.metadata= !{!11}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang", isOptimized: true, emissionKind: FullDebug)
+!1 = !DIFile(filename: "test.cpp", directory: "/path/to")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = distinct !DISubprogram(name: "foo<param>", scope: !1, file: !1, line: 27, type: !6, scopeLine: 27, spFlags: DISPFlagDefinition, unit: !0, templateParams: !8)
+!6 = !DISubroutineType(types: !7)
+!7 = !{null}
+!8 = !{!9}
+!9 = !DITemplateValueParameter(name: "S", type: !10, value: %"class.1" addrspace(1)* bitcast ({ { float, i32, i8 } } addrspace(1)* @extern_const to %"class.1" addrspace(1)*))
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!11 = !{%"class.1" addrspace(1)* bitcast ({ { float, i32, i8 } } addrspace(1)* @extern_const to %"class.1" addrspace(1)*)}