[PATCH] Fix to PR17741 (SLP metadata)

Raul Silvera rsilvera at google.com
Mon Nov 11 22:39:53 PST 2013


Hi nadav, hfinkel,

Currently the SLP vectorizer does not preserve instruction metadata.
This patch adds code to propagate metadata when creating vectorized instructions.

http://llvm-reviews.chandlerc.com/D2147

Files:
  include/llvm/Support/Dwarf.h
  lib/CodeGen/AsmPrinter/DIEHash.cpp
  lib/CodeGen/AsmPrinter/DIEHash.h
  lib/Transforms/Vectorize/SLPVectorizer.cpp
  unittests/CodeGen/DIEHashTest.cpp

Index: include/llvm/Support/Dwarf.h
===================================================================
--- include/llvm/Support/Dwarf.h
+++ include/llvm/Support/Dwarf.h
@@ -141,6 +141,34 @@
   DW_TAG_hi_user = 0xffff
 };
 
+inline bool isType(Tag T) {
+  switch (T) {
+  case DW_TAG_array_type:
+  case DW_TAG_class_type:
+  case DW_TAG_interface_type:
+  case DW_TAG_enumeration_type:
+  case DW_TAG_pointer_type:
+  case DW_TAG_reference_type:
+  case DW_TAG_rvalue_reference_type:
+  case DW_TAG_string_type:
+  case DW_TAG_structure_type:
+  case DW_TAG_subroutine_type:
+  case DW_TAG_union_type:
+  case DW_TAG_ptr_to_member_type:
+  case DW_TAG_set_type:
+  case DW_TAG_subrange_type:
+  case DW_TAG_base_type:
+  case DW_TAG_const_type:
+  case DW_TAG_file_type:
+  case DW_TAG_packed_type:
+  case DW_TAG_volatile_type:
+  case DW_TAG_typedef:
+    return true;
+  default:
+    return false;
+  }
+}
+
 enum Attribute LLVM_ENUM_INT_TYPE(uint16_t) {
   // Attributes
   DW_AT_sibling = 0x01,
Index: lib/CodeGen/AsmPrinter/DIEHash.cpp
===================================================================
--- lib/CodeGen/AsmPrinter/DIEHash.cpp
+++ lib/CodeGen/AsmPrinter/DIEHash.cpp
@@ -384,6 +384,18 @@
   hashAttributes(Attrs, Die.getTag());
 }
 
+void DIEHash::hashNestedType(const DIE &Die, StringRef Name) {
+  // 7.27 Step 7
+  // ... append the letter 'S',
+  addULEB128('S');
+
+  // the tag of C,
+  addULEB128(Die.getTag());
+
+  // and the name.
+  addString(Name);
+}
+
 // Compute the hash of a DIE. This is based on the type signature computation
 // given in section 7.27 of the DWARF4 standard. It is the md5 hash of a
 // flattened description of the DIE.
@@ -398,8 +410,19 @@
   // Then hash each of the children of the DIE.
   for (std::vector<DIE *>::const_iterator I = Die.getChildren().begin(),
                                           E = Die.getChildren().end();
-       I != E; ++I)
+       I != E; ++I) {
+    // 7.27 Step 7
+    // If C is a nested type entry or a member function entry, ...
+    if (isType((*I)->getTag())) {
+      StringRef Name = getDIEStringAttr(**I, dwarf::DW_AT_name);
+      // ... and has a DW_AT_name attribute
+      if (!Name.empty()) {
+        hashNestedType(**I, Name);
+        continue;
+      }
+    }
     computeHash(**I);
+  }
 
   // Following the last (or if there are no children), append a zero byte.
   Hash.update(makeArrayRef((uint8_t)'\0'));
Index: lib/CodeGen/AsmPrinter/DIEHash.h
===================================================================
--- lib/CodeGen/AsmPrinter/DIEHash.h
+++ lib/CodeGen/AsmPrinter/DIEHash.h
@@ -137,6 +137,8 @@
   /// \brief Hashes a reference to a previously referenced type DIE.
   void hashRepeatedTypeReference(dwarf::Attribute Attribute, unsigned DieNumber);
 
+  void hashNestedType(const DIE &Die, StringRef Name);
+
 private:
   MD5 Hash;
   DenseMap<const DIE *, unsigned> Numbering;
Index: lib/Transforms/Vectorize/SLPVectorizer.cpp
===================================================================
--- lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -163,6 +163,39 @@
   return Opcode;
 }
 
+/// \returns \p I after propagating metadata from \p VL.
+static Instruction *propagateMetadata(Instruction *I, ArrayRef<Value *> VL) {
+  Instruction *I0 = dyn_cast<Instruction>(VL[0]);
+  if (!I0) return I;
+
+  SmallVector<std::pair<unsigned, MDNode*>, 4> Metadata;
+  I0->getAllMetadataOtherThanDebugLoc(Metadata);
+
+  for (unsigned i = 0, n = Metadata.size(); i < n; ++i) {
+    unsigned Kind = Metadata[i].first;
+    MDNode *MD = Metadata[i].second;
+
+    for (int i = 1, e = VL.size(); MD && i < e; i++) {
+      Instruction *I = dyn_cast<Instruction>(VL[i]);
+      MDNode *IMD = I->getMetadata(Kind);
+
+      switch(Kind) {
+        default:
+          MD = 0; // Remove unknown metadata
+          break;
+        case LLVMContext::MD_tbaa:
+          MD = MDNode::getMostGenericTBAA(MD, IMD);
+          break;
+        case LLVMContext::MD_fpmath:
+          MD = MDNode::getMostGenericFPMath(MD, IMD);
+          break;
+      }
+    }
+    I->setMetadata(Kind, MD);
+  }
+  return I;
+}
+
 /// \returns The type that all of the values in \p VL have or null if there
 /// are different types.
 static Type* getSameType(ArrayRef<Value *> VL) {
@@ -1471,7 +1504,8 @@
       BinaryOperator *BinOp = cast<BinaryOperator>(VL0);
       Value *V = Builder.CreateBinOp(BinOp->getOpcode(), LHS, RHS);
       E->VectorizedValue = V;
-      return V;
+
+      return propagateMetadata(dyn_cast<Instruction>(V), E->Scalars);
     }
     case Instruction::Load: {
       // Loads are inserted at the head of the tree because we don't want to
@@ -1487,7 +1521,7 @@
       LI = Builder.CreateLoad(VecPtr);
       LI->setAlignment(Alignment);
       E->VectorizedValue = LI;
-      return LI;
+      return propagateMetadata(LI, E->Scalars);
     }
     case Instruction::Store: {
       StoreInst *SI = cast<StoreInst>(VL0);
@@ -1506,7 +1540,7 @@
       StoreInst *S = Builder.CreateStore(VecValue, VecPtr);
       S->setAlignment(Alignment);
       E->VectorizedValue = S;
-      return S;
+      return propagateMetadata(S, E->Scalars);
     }
     default:
     llvm_unreachable("unknown inst");
Index: unittests/CodeGen/DIEHashTest.cpp
===================================================================
--- unittests/CodeGen/DIEHashTest.cpp
+++ unittests/CodeGen/DIEHashTest.cpp
@@ -477,4 +477,23 @@
 
   ASSERT_EQ(0x954e026f01c02529ULL, MD5Res);
 }
+
+// struct { struct bar { }; };
+TEST(DIEHashTest, NestedType) {
+  DIE Unnamed(dwarf::DW_TAG_structure_type);
+  DIEInteger One(1);
+  Unnamed.addValue(dwarf::DW_AT_byte_size, dwarf::DW_FORM_data1, &One);
+
+  DIE *Foo = new DIE(dwarf::DW_TAG_structure_type);
+  DIEString FooStr(&One, "foo");
+  Foo->addValue(dwarf::DW_AT_name, dwarf::DW_FORM_strp, &FooStr);
+  Foo->addValue(dwarf::DW_AT_byte_size, dwarf::DW_FORM_data1, &One);
+
+  Unnamed.addChild(Foo);
+
+  uint64_t MD5Res = DIEHash().computeTypeSignature(Unnamed);
+
+  // The exact same hash GCC produces for this DIE.
+  ASSERT_EQ(0xde8a3b7b43807f4aULL, MD5Res);
+}
 }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2147.1.patch
Type: text/x-patch
Size: 6154 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131111/7896bffb/attachment.bin>


More information about the llvm-commits mailing list