From b6b42e018a808818df9ceecee57ff7b176a68a2b Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Fri, 2 Jun 2017 17:24:26 +0000 Subject: [PATCH] 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) llvm-svn: 304566 --- llvm/include/llvm/ADT/SmallVector.h | 8 ++++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp | 46 ++++++++--------------- llvm/lib/Target/X86/X86ISelLowering.cpp | 4 +- llvm/unittests/ADT/SmallVectorTest.cpp | 10 +++++ 4 files changed, 36 insertions(+), 32 deletions(-) diff --git a/llvm/include/llvm/ADT/SmallVector.h b/llvm/include/llvm/ADT/SmallVector.h index bd24eab93b50..35c255002001 100644 --- a/llvm/include/llvm/ADT/SmallVector.h +++ b/llvm/include/llvm/ADT/SmallVector.h @@ -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 void assign(in_iter in_start, in_iter in_end) { + clear(); + append(in_start, in_end); + } + void assign(std::initializer_list IL) { clear(); append(IL); diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp index 94f5f56ef5a0..6301ae209247 100644 --- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -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> &)> - Callback) { + template 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::writeValueSymbolTableForwardDecl() { 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::writeModuleInfo() { SmallVector 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::writeFunctionLevelValueSymbolTable( 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 Vals; forEachModule( [&](const StringMapEntry> &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); } diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp index 0a41f35f9320..dd2478ff7979 100644 --- a/llvm/lib/Target/X86/X86ISelLowering.cpp +++ b/llvm/lib/Target/X86/X86ISelLowering.cpp @@ -4753,7 +4753,7 @@ static void scaleShuffleMask(int Scale, ArrayRef Mask, SmallVectorImpl &ScaledMask) { assert(0 < Scale && "Unexpected scaling factor"); int NumElts = Mask.size(); - ScaledMask.assign(NumElts * Scale, -1); + ScaledMask.assign(static_cast(NumElts * Scale), -1); for (int i = 0; i != NumElts; ++i) { int M = Mask[i]; @@ -8003,7 +8003,7 @@ static bool is128BitLaneCrossingShuffleMask(MVT VT, ArrayRef Mask) { static bool isRepeatedShuffleMask(unsigned LaneSizeInBits, MVT VT, ArrayRef Mask, SmallVectorImpl &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) { diff --git a/llvm/unittests/ADT/SmallVectorTest.cpp b/llvm/unittests/ADT/SmallVectorTest.cpp index 7367ad470e3a..ca6391024f27 100644 --- a/llvm/unittests/ADT/SmallVectorTest.cpp +++ b/llvm/unittests/ADT/SmallVectorTest.cpp @@ -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");