[llvm] r220645 - Revert "[PBQP] Unique-ptrify some PBQP Metadata structures. No functional change." (r220642)
Lang Hames
lhames at gmail.com
Sun Oct 26 14:26:23 PDT 2014
>From what Dave said it sounds like MSVC just flat out doesn't do this. I
believe the problem is just that it (NodeMetadata) is a member of a
copyable class (Graph::NodeEntry).
That's ok - the other (easier) solution is to just provide a sensible copy
constructor.
- Lang.
On Sun, Oct 26, 2014 at 2:23 PM, Hans Wennborg <hans at chromium.org> wrote:
> I tried commenting out the copy constructor declaration, but doesn't
> help - then it just tries to synthesize it itself. I don't know why
> it's not going for the move constructor and the compiler doesn't
> provide any clue as to who is moving/copying the object.
>
> If it's being used in some STL container or algorithm, maybe msvc's
> library doesn't know that it should move the object instead of copy
> it? :/
>
> On Sun, Oct 26, 2014 at 2:20 PM, Hans Wennborg <hans at chromium.org> wrote:
> > Looks like it's still trying to use the now private copy constructor:
> >
> > [3/5] Building CXX object
> lib\CodeGen\...s\LLVMCodeGen.dir\RegAllocPBQP.cpp.obj
> > FAILED: "C:/Program Files (x86)/CMake 2.8/bin/cmcldeps.exe" CXX
> ..\lib\CodeGen\R
> > egAllocPBQP.cpp
> "lib/CodeGen/CMakeFiles/LLVMCodeGen.dir/RegAllocPBQP.cpp.obj.d"
> > lib\CodeGen\CMakeFiles\LLVMCodeGen.dir\RegAllocPBQP.cpp.obj "Note:
> including fi
> > le: " "C:/Program Files (x86)/Microsoft Visual Studio
> 11.0/VC/bin/cl.exe" C:\PR
> > OGRA~2\MICROS~3.0\VC\bin\cl.exe /nologo /TP /DWIN32 /D_WINDOWS /W3
> /MT /O2 /
> > Ob2 -Ilib\CodeGen -I..\lib\CodeGen -Iinclude -I..\include -UNDEBUG
> -wd4146 -w
> > d4180 -wd4244 -wd4258 -wd4267 -wd4291 -wd4345 -wd4351 -wd4355 -wd4503
> -wd4624 -w
> > d4722 -wd4800 -w14062 -we4238 /EHs-c- /GR- -DGTEST_HAS_RTTI=0
> -D_CRT_NONSTDC_NO
> > _DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE
> -D_CRT_SECURE_N
> > O_WARNINGS -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE
> -D_SCL_SECURE_NO_WARNI
> > NGS -D_VARIADIC_MAX=10 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS
> -D__STDC_
> > LIMIT_MACROS
> /Folib\CodeGen\CMakeFiles\LLVMCodeGen.dir\RegAllocPBQP.cpp.obj /Fdl
> > ib\CodeGen\CMakeFiles\LLVMCodeGen.dir/ -c ..\lib\CodeGen\RegAllocPBQP.cpp
> > d:\src\llvm\include\llvm\codegen\pbqp\Graph.h(100) : error C2248:
> 'llvm::PBQP::R
> > egAlloc::NodeMetadata::NodeMetadata' : cannot access private member
> declared in
> > class 'llvm::PBQP::RegAlloc::NodeMetadata'
> > ..\include\llvm/CodeGen/RegAllocPBQP.h(141) : see declaration of
> 'llvm::
> > PBQP::RegAlloc::NodeMetadata::NodeMetadata'
> > ..\include\llvm/CodeGen/RegAllocPBQP.h(76) : see declaration of
> 'llvm::P
> > BQP::RegAlloc::NodeMetadata'
> > This diagnostic occurred in the compiler generated function
> 'llvm::PBQP:
> > :Graph<SolverT>::NodeEntry::NodeEntry(const
> llvm::PBQP::Graph<SolverT>::NodeEntr
> > y &)'
> > with
> > [
> > SolverT=llvm::PBQP::RegAlloc::RegAllocSolverImpl
> > ]
> > ninja: build stopped: subcommand failed.
> >
> >
> > On Sun, Oct 26, 2014 at 2:09 PM, Lang Hames <lhames at gmail.com> wrote:
> >> Hi Hans,
> >>
> >> Thanks for the insight. I've tried to work around this by explicitly
> >> defining a move constructor (and deleting the copy constructor) in
> r220649.
> >> Hopefully that works.
> >>
> >> Cheers,
> >> Lang.
> >>
> >> On Sun, Oct 26, 2014 at 1:47 PM, Hans Wennborg <hans at chromium.org>
> wrote:
> >>>
> >>> On Sun, Oct 26, 2014 at 1:44 PM, Lang Hames <lhames at gmail.com> wrote:
> >>> > Oh:
> >>> >
> >>> > This diagnostic occurred in the compiler generated function
> >>> > 'llvm::PBQP::RegAlloc::NodeMetadata::NodeMetadata(const
> >>> > llvm::PBQP::RegAlloc::NodeMetadata &)'
> >>> >
> >>> > That explains that, though it's less clear why VC thinks a copy
> >>> > constructor
> >>> > is needed.
> >>> >
> >>> > Will have a fix shortly.
> >>>
> >>> Thanks! I think we've run into similar problems before where some
> >>> object gets moved but VC is synthesizing a copy constructor instead,
> >>> and fails to copy the unique_ptr member.
> >>>
> >>>
> >>> > On Sun, Oct 26, 2014 at 1:36 PM, Lang Hames <lhames at gmail.com>
> wrote:
> >>> >>
> >>> >> That's odd. It's just a unique_ptr to an array - that should be
> >>> >> supported.
> >>> >> I'm going to try #including memory directly and recommit.
> >>> >>
> >>> >> - Lang.
> >>> >>
> >>> >> On Sun, Oct 26, 2014 at 12:50 PM, Hans Wennborg <hans at hanshq.net>
> >>> >> wrote:
> >>> >>>
> >>> >>> Author: hans
> >>> >>> Date: Sun Oct 26 14:50:13 2014
> >>> >>> New Revision: 220645
> >>> >>>
> >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=220645&view=rev
> >>> >>> Log:
> >>> >>> Revert "[PBQP] Unique-ptrify some PBQP Metadata structures. No
> >>> >>> functional
> >>> >>> change." (r220642)
> >>> >>>
> >>> >>> It broke the Windows build:
> >>> >>>
> >>> >>> [1/19] Building CXX object
> >>> >>> lib\CodeGen\CMakeFiles\LLVMCodeGen.dir\RegAllocPBQP.cpp.obj
> >>> >>>
> >>> >>>
> >>> >>>
> C:\bb-win7\ninja-clang-i686-msc17-R\llvm-project\llvm\include\llvm/CodeGen/RegAllocPBQP.h(132)
> >>> >>> : error C2248: 'std::unique_ptr<_Ty>::unique_ptr' : cannot access
> >>> >>> private
> >>> >>> member declared in class 'std::unique_ptr<_Ty>'
> >>> >>>
> >>> >>> with
> >>> >>> [
> >>> >>> _Ty=unsigned int []
> >>> >>> ]
> >>> >>> D:\Program Files (x86)\Microsoft Visual Studio
> >>> >>> 11.0\VC\include\memory(1600) : see declaration of
> >>> >>> 'std::unique_ptr<_Ty>::unique_ptr'
> >>> >>> with
> >>> >>> [
> >>> >>> _Ty=unsigned int []
> >>> >>> ]
> >>> >>> This diagnostic occurred in the compiler generated function
> >>> >>> 'llvm::PBQP::RegAlloc::NodeMetadata::NodeMetadata(const
> >>> >>> llvm::PBQP::RegAlloc::NodeMetadata &)'
> >>> >>>
> >>> >>> Modified:
> >>> >>> llvm/trunk/include/llvm/CodeGen/RegAllocPBQP.h
> >>> >>>
> >>> >>> Modified: llvm/trunk/include/llvm/CodeGen/RegAllocPBQP.h
> >>> >>> URL:
> >>> >>>
> >>> >>>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/RegAllocPBQP.h?rev=220645&r1=220644&r2=220645&view=diff
> >>> >>>
> >>> >>>
> >>> >>>
> ==============================================================================
> >>> >>> --- llvm/trunk/include/llvm/CodeGen/RegAllocPBQP.h (original)
> >>> >>> +++ llvm/trunk/include/llvm/CodeGen/RegAllocPBQP.h Sun Oct 26
> 14:50:13
> >>> >>> 2014
> >>> >>> @@ -62,15 +62,20 @@ public:
> >>> >>> delete[] ColCounts;
> >>> >>> }
> >>> >>>
> >>> >>> + ~MatrixMetadata() {
> >>> >>> + delete[] UnsafeRows;
> >>> >>> + delete[] UnsafeCols;
> >>> >>> + }
> >>> >>> +
> >>> >>> unsigned getWorstRow() const { return WorstRow; }
> >>> >>> unsigned getWorstCol() const { return WorstCol; }
> >>> >>> - const bool* getUnsafeRows() const { return UnsafeRows.get(); }
> >>> >>> - const bool* getUnsafeCols() const { return UnsafeCols.get(); }
> >>> >>> + const bool* getUnsafeRows() const { return UnsafeRows; }
> >>> >>> + const bool* getUnsafeCols() const { return UnsafeCols; }
> >>> >>>
> >>> >>> private:
> >>> >>> unsigned WorstRow, WorstCol;
> >>> >>> - std::unique_ptr<bool[]> UnsafeRows;
> >>> >>> - std::unique_ptr<bool[]> UnsafeCols;
> >>> >>> + bool* UnsafeRows;
> >>> >>> + bool* UnsafeCols;
> >>> >>> };
> >>> >>>
> >>> >>> class NodeMetadata {
> >>> >>> @@ -83,6 +88,7 @@ public:
> >>> >>> NotProvablyAllocatable } ReductionState;
> >>> >>>
> >>> >>> NodeMetadata() : RS(Unprocessed), DeniedOpts(0),
> >>> >>> OptUnsafeEdges(nullptr){}
> >>> >>> + ~NodeMetadata() { delete[] OptUnsafeEdges; }
> >>> >>>
> >>> >>> void setVReg(unsigned VReg) { this->VReg = VReg; }
> >>> >>> unsigned getVReg() const { return VReg; }
> >>> >>> @@ -94,7 +100,7 @@ public:
> >>> >>>
> >>> >>> void setup(const Vector& Costs) {
> >>> >>> NumOpts = Costs.getLength() - 1;
> >>> >>> - OptUnsafeEdges = std::unique_ptr<unsigned[]>(new
> >>> >>> unsigned[NumOpts]());
> >>> >>> + OptUnsafeEdges = new unsigned[NumOpts]();
> >>> >>> }
> >>> >>>
> >>> >>> ReductionState getReductionState() const { return RS; }
> >>> >>> @@ -118,15 +124,15 @@ public:
> >>> >>>
> >>> >>> bool isConservativelyAllocatable() const {
> >>> >>> return (DeniedOpts < NumOpts) ||
> >>> >>> - (std::find(&OptUnsafeEdges[0], &OptUnsafeEdges[NumOpts], 0)
> !=
> >>> >>> - &OptUnsafeEdges[NumOpts]);
> >>> >>> + (std::find(OptUnsafeEdges, OptUnsafeEdges + NumOpts, 0) !=
> >>> >>> + OptUnsafeEdges + NumOpts);
> >>> >>> }
> >>> >>>
> >>> >>> private:
> >>> >>> ReductionState RS;
> >>> >>> unsigned NumOpts;
> >>> >>> unsigned DeniedOpts;
> >>> >>> - std::unique_ptr<unsigned[]> OptUnsafeEdges;
> >>> >>> + unsigned* OptUnsafeEdges;
> >>> >>> unsigned VReg;
> >>> >>> OptionToRegMap OptionRegs;
> >>> >>> };
> >>> >>>
> >>> >>>
> >>> >>> _______________________________________________
> >>> >>> 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/20141026/6dc368bf/attachment.html>
More information about the llvm-commits
mailing list