[llvm] r304566 - Tidy up a bit of r304516, use SmallVector::assign rather than for loop

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 2 10:24:27 PDT 2017


Author: dblaikie
Date: Fri Jun  2 12:24:26 2017
New Revision: 304566

URL: http://llvm.org/viewvc/llvm-project?rev=304566&view=rev
Log:
Tidy up a bit of r304516, use SmallVector::assign rather than for loop

This might give a few better opportunities to optimize these to memcpy
rather than loops - also a few minor cleanups (StringRef-izing,
templating (to avoid std::function indirection), etc).

The SmallVector::assign(iter, iter) could be improved with the use of
SFINAE, but the (iter, iter) ctor and append(iter, iter) need it to and
don't have it - so, workaround it for now rather than bothering with the
added complexity.

(also, as noted in the added FIXME, these assign ops could potentially
be optimized better at least for non-trivially-copyable types)

Modified:
    llvm/trunk/include/llvm/ADT/SmallVector.h
    llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
    llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
    llvm/trunk/unittests/ADT/SmallVectorTest.cpp

Modified: llvm/trunk/include/llvm/ADT/SmallVector.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SmallVector.h?rev=304566&r1=304565&r2=304566&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/SmallVector.h (original)
+++ llvm/trunk/include/llvm/ADT/SmallVector.h Fri Jun  2 12:24:26 2017
@@ -415,6 +415,9 @@ public:
     append(IL.begin(), IL.end());
   }
 
+  // FIXME: Consider assigning over existing elements, rather than clearing &
+  // re-initializing them - for all assign(...) variants.
+
   void assign(size_type NumElts, const T &Elt) {
     clear();
     if (this->capacity() < NumElts)
@@ -423,6 +426,11 @@ public:
     std::uninitialized_fill(this->begin(), this->end(), Elt);
   }
 
+  template <typename in_iter> void assign(in_iter in_start, in_iter in_end) {
+    clear();
+    append(in_start, in_end);
+  }
+
   void assign(std::initializer_list<T> IL) {
     clear();
     append(IL);

Modified: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp?rev=304566&r1=304565&r2=304566&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp (original)
+++ llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp Fri Jun  2 12:24:26 2017
@@ -367,10 +367,7 @@ public:
   /// should be written to the module path string table. This hides the details
   /// of whether they are being pulled from the entire index or just those in a
   /// provided ModuleToSummariesForIndex map.
-  void
-  forEachModule(std::function<
-                void(const StringMapEntry<std::pair<uint64_t, ModuleHash>> &)>
-                    Callback) {
+  template <typename Functor> void forEachModule(Functor Callback) {
     if (ModuleToSummariesForIndex) {
       for (const auto &M : *ModuleToSummariesForIndex) {
         const auto &MPI = Index.modulePaths().find(M.first);
@@ -986,19 +983,18 @@ void ModuleBitcodeWriter::writeValueSymb
 enum StringEncoding { SE_Char6, SE_Fixed7, SE_Fixed8 };
 
 /// Determine the encoding to use for the given string name and length.
-static StringEncoding getStringEncoding(const char *Str, unsigned StrLen) {
+static StringEncoding getStringEncoding(StringRef Str) {
   bool isChar6 = true;
-  for (const char *C = Str, *E = C + StrLen; C != E; ++C) {
+  for (char C : Str) {
     if (isChar6)
-      isChar6 = BitCodeAbbrevOp::isChar6(*C);
-    if ((unsigned char)*C & 128)
+      isChar6 = BitCodeAbbrevOp::isChar6(C);
+    if ((unsigned char)C & 128)
       // don't bother scanning the rest.
       return SE_Fixed8;
   }
   if (isChar6)
     return SE_Char6;
-  else
-    return SE_Fixed7;
+  return SE_Fixed7;
 }
 
 /// Emit top-level description of module, including target triple, inline asm,
@@ -1091,8 +1087,7 @@ void ModuleBitcodeWriter::writeModuleInf
   SmallVector<unsigned, 64> Vals;
   // Emit the module's source file name.
   {
-    StringEncoding Bits = getStringEncoding(M.getSourceFileName().data(),
-                                            M.getSourceFileName().size());
+    StringEncoding Bits = getStringEncoding(M.getSourceFileName());
     BitCodeAbbrevOp AbbrevOpToUse = BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 8);
     if (Bits == SE_Char6)
       AbbrevOpToUse = BitCodeAbbrevOp(BitCodeAbbrevOp::Char6);
@@ -2808,8 +2803,7 @@ void ModuleBitcodeWriter::writeFunctionL
 
   for (const ValueName &Name : VST) {
     // Figure out the encoding to use for the name.
-    StringEncoding Bits =
-        getStringEncoding(Name.getKeyData(), Name.getKeyLength());
+    StringEncoding Bits = getStringEncoding(Name.getKey());
 
     unsigned AbbrevToUse = VST_ENTRY_8_ABBREV;
     NameVals.push_back(VE.getValueID(Name.getValue()));
@@ -3169,33 +3163,25 @@ void IndexBitcodeWriter::writeModStrings
   SmallVector<unsigned, 64> Vals;
   forEachModule(
       [&](const StringMapEntry<std::pair<uint64_t, ModuleHash>> &MPSE) {
-        StringEncoding Bits =
-            getStringEncoding(MPSE.getKey().data(), MPSE.getKey().size());
+        StringRef Key = MPSE.getKey();
+        const auto &Value = MPSE.getValue();
+        StringEncoding Bits = getStringEncoding(Key);
         unsigned AbbrevToUse = Abbrev8Bit;
         if (Bits == SE_Char6)
           AbbrevToUse = Abbrev6Bit;
         else if (Bits == SE_Fixed7)
           AbbrevToUse = Abbrev7Bit;
 
-        Vals.push_back(MPSE.getValue().first);
-
-        for (const auto P : MPSE.getKey())
-          Vals.push_back((unsigned char)P);
+        Vals.push_back(Value.first);
+        Vals.append(Key.begin(), Key.end());
 
         // Emit the finished record.
         Stream.EmitRecord(bitc::MST_CODE_ENTRY, Vals, AbbrevToUse);
 
-        Vals.clear();
         // Emit an optional hash for the module now
-        auto &Hash = MPSE.getValue().second;
-        bool AllZero =
-            true; // Detect if the hash is empty, and do not generate it
-        for (auto Val : Hash) {
-          if (Val)
-            AllZero = false;
-          Vals.push_back(Val);
-        }
-        if (!AllZero) {
+        const auto &Hash = Value.second;
+        if (llvm::any_of(Hash, [](uint32_t H) { return H; })) {
+          Vals.assign(Hash.begin(), Hash.end());
           // Emit the hash record.
           Stream.EmitRecord(bitc::MST_CODE_HASH, Vals, AbbrevHash);
         }

Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=304566&r1=304565&r2=304566&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
+++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Fri Jun  2 12:24:26 2017
@@ -4753,7 +4753,7 @@ static void scaleShuffleMask(int Scale,
                              SmallVectorImpl<int> &ScaledMask) {
   assert(0 < Scale && "Unexpected scaling factor");
   int NumElts = Mask.size();
-  ScaledMask.assign(NumElts * Scale, -1);
+  ScaledMask.assign(static_cast<size_t>(NumElts * Scale), -1);
 
   for (int i = 0; i != NumElts; ++i) {
     int M = Mask[i];
@@ -8003,7 +8003,7 @@ static bool is128BitLaneCrossingShuffleM
 static bool isRepeatedShuffleMask(unsigned LaneSizeInBits, MVT VT,
                                   ArrayRef<int> Mask,
                                   SmallVectorImpl<int> &RepeatedMask) {
-  int LaneSize = LaneSizeInBits / VT.getScalarSizeInBits();
+  auto LaneSize = LaneSizeInBits / VT.getScalarSizeInBits();
   RepeatedMask.assign(LaneSize, -1);
   int Size = Mask.size();
   for (int i = 0; i < Size; ++i) {

Modified: llvm/trunk/unittests/ADT/SmallVectorTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/SmallVectorTest.cpp?rev=304566&r1=304565&r2=304566&view=diff
==============================================================================
--- llvm/trunk/unittests/ADT/SmallVectorTest.cpp (original)
+++ llvm/trunk/unittests/ADT/SmallVectorTest.cpp Fri Jun  2 12:24:26 2017
@@ -424,6 +424,16 @@ TYPED_TEST(SmallVectorTest, AssignTest)
   this->assertValuesInOrder(this->theVector, 2u, 77, 77);
 }
 
+// Assign test
+TYPED_TEST(SmallVectorTest, AssignRangeTest) {
+  SCOPED_TRACE("AssignTest");
+
+  this->theVector.push_back(Constructable(1));
+  int arr[] = {1, 2, 3};
+  this->theVector.assign(std::begin(arr), std::end(arr));
+  this->assertValuesInOrder(this->theVector, 3u, 1, 2, 3);
+}
+
 // Move-assign test
 TYPED_TEST(SmallVectorTest, MoveAssignTest) {
   SCOPED_TRACE("MoveAssignTest");




More information about the llvm-commits mailing list