[llvm] r293017 - Rewind instantiations of OuterAnalysisManagerProxy in r289317, r291651, and r291662.

Hans Wennborg via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 7 11:20:42 PST 2017 

Merged in r294335.

Thanks everyone!

On Mon, Feb 6, 2017 at 9:56 PM, NAKAMURA Takumi <geek4civic at gmail.com> wrote:
> It works.
>
> On Tue, Feb 7, 2017 at 11:03 AM Chandler Carruth <chandlerc at gmail.com>
> wrote:
>>
>> Take a look at r294267 to see the underlying issue. This patch, indeed,
>> papers over a really serious bug by "disabling" the extern templates in
>> effect.
>>
>> Hans, it would be good to get r294267 into the branch. It might be easiest
>> to pull this patch and that one to simplify merging.
>>
>> On Mon, Feb 6, 2017 at 4:06 PM Chandler Carruth <chandlerc at gmail.com>
>> wrote:
>>>
>>> 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

More information about the llvm-commits mailing list