[llvm] r218514 - Revert "Store TypeUnits in a SmallVector<DWARFUnitSection> instead of a single DWARFUnitSection."
Frédéric Riss
friss at apple.com
Fri Sep 26 23:07:34 PDT 2014
> On 26 Sep 2014, at 23:39, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>
>>
>> On Sep 26, 2014, at 6:49 AM, Frédéric Riss <friss at apple.com> wrote:
>>
>>>
>>> On 26 Sep 2014, at 14:34, Frederic Riss <friss at apple.com> wrote:
>>>
>>> Author: friss
>>> Date: Fri Sep 26 07:34:06 2014
>>> New Revision: 218514
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=218514&view=rev
>>> Log:
>>> Revert "Store TypeUnits in a SmallVector<DWARFUnitSection> instead of a single DWARFUnitSection."
>>>
>>> This reverts commit r218513.
>>>
>>> Buildbots using libstdc++ issue an error when trying to copy
>>> SmallVector<std::unique_ptr<>>. Revert the commit until we have a fix.
>>
>> This wasn’t only libstdc++ in fact, MSVC also chocked and I have one libcxx bot that also failed.
>>
>> David, if you look at the commit, you’ll see that I didn’t replace the SmallVectors we talked about by std::vectors. I tried it, but it failed in a very similar way to the builders. I thought this was some libcxx or SmallVector issue and I was going to report it with a reproducer:
>>
>> #include <vector>
>> #include <memory>
>> #include <llvm/ADT/SmallVector.h>
>>
>> typedef llvm::SmallVector<std::unique_ptr<int>,1> vec_type;
>> std::vector<vec_type> vecvec;
>>
>> void foo() {
>> vecvec.push_back(vec_type());
>> }
>>
>> Is this code somehow wrong?
>> This fails to compile for me because the move semantics are lost somewhere and the implicitly deleted unique_ptr copy constructor is called. Note that other permutations of vector and SmallVector seem to work.
>>
>> If I add explicit default and move constructors to DWARFUnitSection, then I am able to replace the SmallVectors by vectors as we discussed, but I don’t understand why this is necessary. Maybe this would also fix the issue seen on the builders, but in either case I can’t see why the compiler generated constructors don’t work.
>>
>> For the record, here are some builder logs:
>> http://lab.llvm.org:8011/builders/clang-native-arm-cortex-a9/builds/21968/steps/compile/logs/stdio
>
> I looked at this one. It looks to me like the compiler hasn’t generated
> a move constructor for `DWARFUnitSection`. I knew MSVC had issues like
> this, but maybe GCC has trouble to. We can’t really trust compilers to
> auto-generate C++11 constructors properly.
By ‘compilers’ you mean clang too ? because there was one clang++ failure in the logs I provided (on freebsd). I suppose it can be an old version that had the same kind of issues.
> Looking at the code, you probably just need something like this:
>
> DwarfUnitSection(DwarfUnitSection &&X)
> : SmallVector<std::unique_ptr<UnitType>>(std::move(X)) {}
Yeah, and also add an empty default constructor. I just wanted to make sure that the code in itself wasn’t flawed in a way that I didn’t understand.
Thanks for looking into it!
Fred
> If MSVC supported it, you could do this instead:
>
> DwarfUnitSection(DwarfUnitSection &&) = default;
>
>> http://lab.llvm.org:8011/builders/lld-x86_64-freebsd/builds/8474/steps/build_Lld/logs/stdio
>> http://lab.llvm.org:8011/builders/lld-x86_64-win7/builds/13282/steps/build_Lld/logs/stdio
>>
>> Fred
>>
>>> Modified:
>>> llvm/trunk/lib/DebugInfo/DWARFContext.cpp
>>> llvm/trunk/lib/DebugInfo/DWARFContext.h
>>>
>>> Modified: llvm/trunk/lib/DebugInfo/DWARFContext.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARFContext.cpp?rev=218514&r1=218513&r2=218514&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/DebugInfo/DWARFContext.cpp (original)
>>> +++ llvm/trunk/lib/DebugInfo/DWARFContext.cpp Fri Sep 26 07:34:06 2014
>>> @@ -86,17 +86,15 @@ void DWARFContext::dump(raw_ostream &OS,
>>>
>>> if ((DumpType == DIDT_All || DumpType == DIDT_Types) && getNumTypeUnits()) {
>>> OS << "\n.debug_types contents:\n";
>>> - for (const auto &TUS : type_unit_sections())
>>> - for (const auto &TU : TUS)
>>> - TU->dump(OS);
>>> + for (const auto &TU : type_units())
>>> + TU->dump(OS);
>>> }
>>>
>>> if ((DumpType == DIDT_All || DumpType == DIDT_TypesDwo) &&
>>> getNumDWOTypeUnits()) {
>>> OS << "\n.debug_types.dwo contents:\n";
>>> - for (const auto &DWOTUS : dwo_type_unit_sections())
>>> - for (const auto &DWOTU : DWOTUS)
>>> - DWOTU->dump(OS);
>>> + for (const auto &DWOTU : dwo_type_units())
>>> + DWOTU->dump(OS);
>>> }
>>>
>>> if (DumpType == DIDT_All || DumpType == DIDT_Loc) {
>>> @@ -339,17 +337,15 @@ void DWARFContext::parseTypeUnits() {
>>> uint32_t offset = 0;
>>> const DataExtractor &DIData =
>>> DataExtractor(I.second.Data, isLittleEndian(), 0);
>>> - TUs.push_back(DWARFUnitSection<DWARFTypeUnit>());
>>> - auto &TUS = TUs.back();
>>> while (DIData.isValidOffset(offset)) {
>>> std::unique_ptr<DWARFTypeUnit> TU(new DWARFTypeUnit(*this,
>>> getDebugAbbrev(), I.second.Data, getRangeSection(),
>>> getStringSection(), StringRef(), getAddrSection(),
>>> - &I.second.Relocs, isLittleEndian(), TUS));
>>> + &I.second.Relocs, isLittleEndian(), TUs));
>>> if (!TU->extract(DIData, &offset))
>>> break;
>>> - TUS.push_back(std::move(TU));
>>> - offset = TUS.back()->getNextUnitOffset();
>>> + TUs.push_back(std::move(TU));
>>> + offset = TUs.back()->getNextUnitOffset();
>>> }
>>> }
>>> }
>>> @@ -380,17 +376,15 @@ void DWARFContext::parseDWOTypeUnits() {
>>> uint32_t offset = 0;
>>> const DataExtractor &DIData =
>>> DataExtractor(I.second.Data, isLittleEndian(), 0);
>>> - DWOTUs.push_back(DWARFUnitSection<DWARFTypeUnit>());
>>> - auto &TUS = DWOTUs.back();
>>> while (DIData.isValidOffset(offset)) {
>>> std::unique_ptr<DWARFTypeUnit> TU(new DWARFTypeUnit(*this,
>>> getDebugAbbrevDWO(), I.second.Data, getRangeDWOSection(),
>>> getStringDWOSection(), getStringOffsetDWOSection(), getAddrSection(),
>>> - &I.second.Relocs, isLittleEndian(), TUS));
>>> + &I.second.Relocs, isLittleEndian(), DWOTUs));
>>> if (!TU->extract(DIData, &offset))
>>> break;
>>> - TUS.push_back(std::move(TU));
>>> - offset = TUS.back()->getNextUnitOffset();
>>> + DWOTUs.push_back(std::move(TU));
>>> + offset = DWOTUs.back()->getNextUnitOffset();
>>> }
>>> }
>>> }
>>>
>>> Modified: llvm/trunk/lib/DebugInfo/DWARFContext.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARFContext.h?rev=218514&r1=218513&r2=218514&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/DebugInfo/DWARFContext.h (original)
>>> +++ llvm/trunk/lib/DebugInfo/DWARFContext.h Fri Sep 26 07:34:06 2014
>>> @@ -30,7 +30,7 @@ namespace llvm {
>>> class DWARFContext : public DIContext {
>>>
>>> DWARFUnitSection<DWARFCompileUnit> CUs;
>>> - SmallVector<DWARFUnitSection<DWARFTypeUnit>,1> TUs;
>>> + DWARFUnitSection<DWARFTypeUnit> TUs;
>>> std::unique_ptr<DWARFDebugAbbrev> Abbrev;
>>> std::unique_ptr<DWARFDebugLoc> Loc;
>>> std::unique_ptr<DWARFDebugAranges> Aranges;
>>> @@ -38,7 +38,7 @@ class DWARFContext : public DIContext {
>>> std::unique_ptr<DWARFDebugFrame> DebugFrame;
>>>
>>> DWARFUnitSection<DWARFCompileUnit> DWOCUs;
>>> - SmallVector<DWARFUnitSection<DWARFTypeUnit>,1> DWOTUs;
>>> + DWARFUnitSection<DWARFTypeUnit> DWOTUs;
>>> std::unique_ptr<DWARFDebugAbbrev> AbbrevDWO;
>>> std::unique_ptr<DWARFDebugLocDWO> LocDWO;
>>>
>>> @@ -77,7 +77,6 @@ public:
>>>
>>> typedef DWARFUnitSection<DWARFCompileUnit>::iterator_range cu_iterator_range;
>>> typedef DWARFUnitSection<DWARFTypeUnit>::iterator_range tu_iterator_range;
>>> - typedef iterator_range<SmallVectorImpl<DWARFUnitSection<DWARFTypeUnit>>::iterator> tu_section_iterator_range;
>>>
>>> /// Get compile units in this context.
>>> cu_iterator_range compile_units() {
>>> @@ -86,9 +85,9 @@ public:
>>> }
>>>
>>> /// Get type units in this context.
>>> - tu_section_iterator_range type_unit_sections() {
>>> + tu_iterator_range type_units() {
>>> parseTypeUnits();
>>> - return tu_section_iterator_range(TUs.begin(), TUs.end());
>>> + return tu_iterator_range(TUs.begin(), TUs.end());
>>> }
>>>
>>> /// Get compile units in the DWO context.
>>> @@ -98,9 +97,9 @@ public:
>>> }
>>>
>>> /// Get type units in the DWO context.
>>> - tu_section_iterator_range dwo_type_unit_sections() {
>>> + tu_iterator_range dwo_type_units() {
>>> parseDWOTypeUnits();
>>> - return tu_section_iterator_range(DWOTUs.begin(), DWOTUs.end());
>>> + return tu_iterator_range(DWOTUs.begin(), DWOTUs.end());
>>> }
>>>
>>> /// Get the number of compile units in this context.
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140927/e8653e8f/attachment.html>
More information about the llvm-commits
mailing list