[llvm] r293017 - Rewind instantiations of OuterAnalysisManagerProxy in r289317, r291651, and r291662.
Chandler Carruth via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 6 15:19:32 PST 2017
I don't think this is true.
I can definitely query clang, and right now, the *only* usage of the extern
template declarations you added in this patch are the explicit
instantiations. No other code uses them.
So what this patch is doing is disabling the extern template entirely for
all practical purposes. It is possible that this works around a bug, but it
is not the correct fix. We need to find the correct fix for this issue.
On Mon, Feb 6, 2017 at 3:17 PM NAKAMURA Takumi via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Chandler, even Clang doesn't instantiate Key as expected.
> Should we wait for clang (and gcc) fixed? For now, I hope to let trunk
> just working.
>
> On Tue, Feb 7, 2017 at 8:12 AM Chandler Carruth <chandlerc at gmail.com>
> wrote:
>
> Sorry my brain was filled with another set of problems, but trying to
> cycle back to this....
>
> I totally believe that there is a real bug to solve here, but this patch
> is not the fix.
>
> Let's look at the types themselves we now have in tree:
>
> ```
> extern template class OuterAnalysisManagerProxy<ModuleAnalysisManager,
> LazyCallGraph::SCC>;
> ```
> vs.
> ```
> typedef OuterAnalysisManagerProxy<ModuleAnalysisManager,
> LazyCallGraph::SCC, LazyCallGraph &> ModuleAnaylsisManagerCGSCCProxy;
> ```
>
> These are *not the same types* and they are explicitly documented that
> they should be the same types.
>
> This patch is not correct. I'm happy to try to solve the bug that you are
> hitting, but this is regressing every other platform that has a correct
> compiler because the extern template is for *the wrong type*.
>
> -Chandler
>
>
> On Mon, Feb 6, 2017 at 3:07 PM NAKAMURA Takumi via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
> I would like you to pull it.
>
> See also;
> http://bb.pgr.jp/builders/ninja-clang-x64-mingw64-RA/builds/15598
>
> On Sat, Feb 4, 2017 at 6:59 AM Hans Wennborg <hans at chromium.org> wrote:
>
> Hmm, what's the status here? It seems this change is still in trunk.
> Should it be merged?
>
> On Mon, Jan 30, 2017 at 8:03 AM, NAKAMURA Takumi <geek4civic at gmail.com>
> wrote:
> > Chandler,
> >
> > I am confused what were correct.
> >
> > Before r293017, they are instantiated explicitly,
> > llvm::OuterAnalysisManagerProxy<llvm::AnalysisManager<llvm::Module>,
> > llvm::LazyCallGraph::SCC, llvm::LazyCallGraph&>::Key
> > llvm::OuterAnalysisManagerProxy<llvm::AnalysisManager<llvm::Function>,
> > llvm::Loop, llvm::LoopStandardAnalysisResults&>::Key
> >
> > They are not referred outside of libLLVMAnalysis.so.
> > Besides, linking can be done successfully even if their instantiations
> (in
> > Analysis/*.cpp) are pruned.
> >
> > I am dubious it would be correct.
> > I have confirmed on x86_64-linux with clang++-trunk,
> BUILD_SHARED_LIBS=ON.
> >
> > I just would like to instantiate OuterAnalysisManagerProxy::Key
> explicitly.
> > Let me know if you have better idea. Just reverting would make things
> worse.
> >
> > FYI, failure log is here;
> > http://bb.pgr.jp/builders/ninja-clang-x64-mingw64-RA/builds/15409
> >
> > On Mon, Jan 30, 2017 at 8:05 AM Chandler Carruth <chandlerc at gmail.com>
> > wrote:
> >>
> >> This change is not correct at all. Please revert as it causes other
> >> platforms to no longer have the necessary extern template declaration.
> What
> >> failure are you trying to address?
> >>
> >>
> >> On Sun, Jan 29, 2017, 14:03 NAKAMURA Takumi <geek4civic at gmail.com>
> wrote:
> >>>
> >>> Hans, could you pull this into 4.0, please?
> >>>
> >>> 2017年1月25日(水) 13:37 NAKAMURA Takumi via llvm-commits
> >>> <llvm-commits at lists.llvm.org>:
> >>>>
> >>>> Author: chapuni
> >>>> Date: Tue Jan 24 22:26:29 2017
> >>>> New Revision: 293017
> >>>>
> >>>> URL: http://llvm.org/viewvc/llvm-project?rev=293017&view=rev
> >>>> Log:
> >>>> Rewind instantiations of OuterAnalysisManagerProxy in r289317,
> r291651,
> >>>> and r291662.
> >>>>
> >>>> I found root class should be instantiated for variadic tempate to
> >>>> instantiate static member explicitly.
> >>>>
> >>>> This will fix failures in mingw DLL build.
> >>>>
> >>>> Modified:
> >>>> llvm/trunk/include/llvm/Analysis/CGSCCPassManager.h
> >>>> llvm/trunk/include/llvm/Analysis/LoopAnalysisManager.h
> >>>> llvm/trunk/lib/Analysis/CGSCCPassManager.cpp
> >>>> llvm/trunk/lib/Analysis/LoopAnalysisManager.cpp
> >>>>
> >>>> Modified: llvm/trunk/include/llvm/Analysis/CGSCCPassManager.h
> >>>> URL:
> >>>>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/CGSCCPassManager.h?rev=293017&r1=293016&r2=293017&view=diff
> >>>>
> >>>>
> ==============================================================================
> >>>> --- llvm/trunk/include/llvm/Analysis/CGSCCPassManager.h (original)
> >>>> +++ llvm/trunk/include/llvm/Analysis/CGSCCPassManager.h Tue Jan 24
> >>>> 22:26:29 2017
> >>>> @@ -191,8 +191,8 @@ CGSCCAnalysisManagerModuleProxy::run(Mod
> >>>> // template.
> >>>> extern template class InnerAnalysisManagerProxy<CGSCCAnalysisManager,
> >>>> Module>;
> >>>>
> >>>> -extern template class OuterAnalysisManagerProxy<
> >>>> - ModuleAnalysisManager, LazyCallGraph::SCC, LazyCallGraph &>;
> >>>> +extern template class
> OuterAnalysisManagerProxy<ModuleAnalysisManager,
> >>>> + LazyCallGraph::SCC>;
> >>>> /// A proxy from a \c ModuleAnalysisManager to an \c SCC.
> >>>> typedef OuterAnalysisManagerProxy<ModuleAnalysisManager,
> >>>> LazyCallGraph::SCC,
> >>>> LazyCallGraph &>
> >>>>
> >>>> Modified: llvm/trunk/include/llvm/Analysis/LoopAnalysisManager.h
> >>>> URL:
> >>>>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/LoopAnalysisManager.h?rev=293017&r1=293016&r2=293017&view=diff
> >>>>
> >>>>
> ==============================================================================
> >>>> --- llvm/trunk/include/llvm/Analysis/LoopAnalysisManager.h (original)
> >>>> +++ llvm/trunk/include/llvm/Analysis/LoopAnalysisManager.h Tue Jan 24
> >>>> 22:26:29 2017
> >>>> @@ -141,8 +141,7 @@ LoopAnalysisManagerFunctionProxy::run(Fu
> >>>> // template.
> >>>> extern template class InnerAnalysisManagerProxy<LoopAnalysisManager,
> >>>> Function>;
> >>>>
> >>>> -extern template class
> >>>> OuterAnalysisManagerProxy<FunctionAnalysisManager, Loop,
> >>>> -
> >>>> LoopStandardAnalysisResults &>;
> >>>> +extern template class
> >>>> OuterAnalysisManagerProxy<FunctionAnalysisManager, Loop>;
> >>>> /// A proxy from a \c FunctionAnalysisManager to a \c Loop.
> >>>> typedef OuterAnalysisManagerProxy<FunctionAnalysisManager, Loop,
> >>>> LoopStandardAnalysisResults &>
> >>>>
> >>>> Modified: llvm/trunk/lib/Analysis/CGSCCPassManager.cpp
> >>>> URL:
> >>>>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/CGSCCPassManager.cpp?rev=293017&r1=293016&r2=293017&view=diff
> >>>>
> >>>>
> ==============================================================================
> >>>> --- llvm/trunk/lib/Analysis/CGSCCPassManager.cpp (original)
> >>>> +++ llvm/trunk/lib/Analysis/CGSCCPassManager.cpp Tue Jan 24 22:26:29
> >>>> 2017
> >>>> @@ -24,7 +24,7 @@ template class PassManager<LazyCallGraph
> >>>> LazyCallGraph &, CGSCCUpdateResult &>;
> >>>> template class InnerAnalysisManagerProxy<CGSCCAnalysisManager,
> Module>;
> >>>> template class OuterAnalysisManagerProxy<ModuleAnalysisManager,
> >>>> - LazyCallGraph::SCC,
> >>>> LazyCallGraph &>;
> >>>> + LazyCallGraph::SCC>;
> >>>> template class OuterAnalysisManagerProxy<CGSCCAnalysisManager,
> >>>> Function>;
> >>>>
> >>>> /// Explicitly specialize the pass manager run method to handle call
> >>>> graph
> >>>>
> >>>> Modified: llvm/trunk/lib/Analysis/LoopAnalysisManager.cpp
> >>>> URL:
> >>>>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/LoopAnalysisManager.cpp?rev=293017&r1=293016&r2=293017&view=diff
> >>>>
> >>>>
> ==============================================================================
> >>>> --- llvm/trunk/lib/Analysis/LoopAnalysisManager.cpp (original)
> >>>> +++ llvm/trunk/lib/Analysis/LoopAnalysisManager.cpp Tue Jan 24
> 22:26:29
> >>>> 2017
> >>>> @@ -23,8 +23,7 @@ namespace llvm {
> >>>> template class AllAnalysesOn<Loop>;
> >>>> template class AnalysisManager<Loop, LoopStandardAnalysisResults &>;
> >>>> template class InnerAnalysisManagerProxy<LoopAnalysisManager,
> >>>> Function>;
> >>>> -template class OuterAnalysisManagerProxy<FunctionAnalysisManager,
> Loop,
> >>>> - LoopStandardAnalysisResults
> >>>> &>;
> >>>> +template class OuterAnalysisManagerProxy<FunctionAnalysisManager,
> >>>> Loop>;
> >>>>
> >>>> bool LoopAnalysisManagerFunctionProxy::Result::invalidate(
> >>>> Function &F, const PreservedAnalyses &PA,
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> llvm-commits mailing list
> >>>> llvm-commits at lists.llvm.org
> >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170206/9fb8f742/attachment.html>
More information about the llvm-commits
mailing list