[PATCH] D140949: [MemProf] Context disambiguation cloning pass [patch 2/3]

Sat Apr 22 18:49:43 PDT 2023

aganea added inline comments.

================
Comment at: llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp:2004
+  // that is arbitrary, and we assert in that case below.
+  constexpr unsigned AllocTypeCloningPriority[] = {/*None*/ 3, /*NotCold*/ 4,
+                                                   /*Cold*/ 1,
----------------
tejohnson wrote:
> tejohnson wrote:
> > aganea wrote:
> > > Hello @tejohnson! Quite note - This patch fails on Windows with MSVC. Historically, MSVC has troubles capturing constexpr variables in lambdas (on L2018). Changing to `const` locally works. See diagnostics:
> > > ```
> > > FAILED: lib/Transforms/IPO/CMakeFiles/LLVMipo.dir/MemProfContextDisambiguation.cpp.obj
> > > C:\PROGRA~1\MIB055~1\2022\PROFES~1\VC\Tools\MSVC\1433~1.316\bin\HostX64\x64\cl.exe  /nologo /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 -D_LIBCPP_ENABLE_ASSERTIONS -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -IC:\git\llvm-project\stage1_msvc_debug\lib\Transforms\IPO -IC:\git\llvm-project\llvm\lib\Transforms\IPO -IC:\git\llvm-project\stage1_msvc_debug\include -IC:\git\llvm-project\llvm\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Oi /bigobj /permissive- /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd5105 -wd4324 -w14062 -we4238 /MTd /Zi /Ob0 /Od /RTC1  /EHs-c- /GR- -std:c++17 /showIncludes /Folib\Transforms\IPO\CMakeFiles\LLVMipo.dir\MemProfContextDisambiguation.cpp.obj /Fdlib\Transforms\IPO\CMakeFiles\LLVMipo.dir\LLVMipo.pdb /FS -c C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp
> > > C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp(2018): error C2326: 'bool CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>::identifyClones::<lambda_1>::operator ()(const std::shared_ptr<CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>::ContextEdge> &,const std::shared_ptr<CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>::ContextEdge> &) const': function cannot access 'AllocTypeCloningPriority'
> > > C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp(1946): note: while compiling class template member function 'void CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>::identifyClones(CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>::ContextNode *,llvm::DenseSet<const CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>::ContextNode *,llvm::DenseMapInfo<ValueT,void>> &)'
> > >         with
> > >         [
> > >             ValueT=const CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>::ContextNode *
> > >         ]
> > > C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp(1941): note: see reference to function template instantiation 'void CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>::identifyClones(CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>::ContextNode *,llvm::DenseSet<const CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>::ContextNode *,llvm::DenseMapInfo<ValueT,void>> &)' being compiled
> > >         with
> > >         [
> > >             ValueT=const CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>::ContextNode *
> > >         ]
> > > C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp(1938): note: while compiling class template member function 'void CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>::identifyClones(void)'
> > > C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp(2139): note: see reference to function template instantiation 'void CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>::identifyClones(void)' being compiled
> > > C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp(2127): note: while compiling class template member function 'bool CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>::process(void)'
> > > C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp(2159): note: see reference to function template instantiation 'bool CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>::process(void)' being compiled
> > > C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp(470): note: see reference to class template instantiation 'CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>' being compiled
> > > C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp(2019): error C2326: 'bool CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>::identifyClones::<lambda_1>::operator ()(const std::shared_ptr<CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>::ContextEdge> &,const std::shared_ptr<CallsiteContextGraph<ModuleCallsiteContextGraph,llvm::Function,llvm::Instruction *>::ContextEdge> &) const': function cannot access 'AllocTypeCloningPriority'
> > > C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp(2018): error C2326: 'bool CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>::identifyClones::<lambda_1>::operator ()(const std::shared_ptr<CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>::ContextEdge> &,const std::shared_ptr<CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>::ContextEdge> &) const': function cannot access 'AllocTypeCloningPriority'
> > > C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp(1946): note: while compiling class template member function 'void CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>::identifyClones(CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>::ContextNode *,llvm::DenseSet<const CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>::ContextNode *,llvm::DenseMapInfo<ValueT,void>> &)'
> > >         with
> > >         [
> > >             ValueT=const CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>::ContextNode *
> > >         ]
> > > C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp(1941): note: see reference to function template instantiation 'void CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>::identifyClones(CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>::ContextNode *,llvm::DenseSet<const CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>::ContextNode *,llvm::DenseMapInfo<ValueT,void>> &)' being compiled
> > >         with
> > >         [
> > >             ValueT=const CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>::ContextNode *
> > >         ]
> > > C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp(1938): note: while compiling class template member function 'void CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>::identifyClones(void)'
> > > C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp(2139): note: see reference to function template instantiation 'void CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>::identifyClones(void)' being compiled
> > > C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp(2127): note: while compiling class template member function 'bool CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>::process(void)'
> > > C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp(2176): note: see reference to function template instantiation 'bool CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>::process(void)' being compiled
> > > C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp(515): note: see reference to class template instantiation 'CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>' being compiled
> > > C:\git\llvm-project\llvm\lib\Transforms\IPO\MemProfContextDisambiguation.cpp(2019): error C2326: 'bool CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>::identifyClones::<lambda_1>::operator ()(const std::shared_ptr<CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>::ContextEdge> &,const std::shared_ptr<CallsiteContextGraph<IndexCallsiteContextGraph,llvm::FunctionSummary,IndexCall>::ContextEdge> &) const': function cannot access 'AllocTypeCloningPriority'
> > > ```
> > Thanks for the report. I didn't see any bot failure reports - do you know if this showed up on one?
> > 
> > Just to confirm, I should remove the constexpr and make it:
> > ```
> > const unsigned AllocTypeCloningPriority[] = ...
> > ```
> > ?
> Also, just to confirm, is this is happening with the most recent version of the patch that was committed (a104e27030587507a711cef0e2b0ddb447fe68fe)? A number of compilers complained about the lambda capture of this constexpr before I changed it slightly.
Yes to both questions. From quickly browsing through the builders configs it seems none of them use MSVC 2022, where the issue happens.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140949/new/

https://reviews.llvm.org/D140949