[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 16:06:33 PST 2017


We've found the actual source error and I'm working on a patch that reverts
this and fixes it properly.

On Mon, Feb 6, 2017 at 3:20 PM Chandler Carruth <chandlerc at gmail.com> wrote:

> Are there any reproduction steps that don't involve mingw?
>
> On Mon, Feb 6, 2017 at 3:19 PM Chandler Carruth <chandlerc at gmail.com>
> wrote:
>
> 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/20170207/4f46a7ea/attachment.html>


More information about the llvm-commits mailing list