Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
| polly/lib/Support/RegisterPasses.cpp | ||
|---|---|---|
| 333–340 | The legacy pass manger is till present and people are still using it (which includes me). That is, the old functionality should not be removed, especially without giving any notification (such as report_fatal_error as with the NPM). In other places, common code was extracted into a separate function such as runSimplifyUsingNPM or wrapper passes provided. Could you do the same? | |
| polly/include/polly/ScopGraphPrinter.h | ||
|---|---|---|
| 53 | I realized that this specialization doesn't take effect. I should use DOTGraphTraits<ScopDetection>. | |
| polly/include/polly/ScopGraphPrinter.h | ||
|---|---|---|
| 53 | Are you going to make this change? | |
| polly/include/polly/ScopGraphPrinter.h | ||
|---|---|---|
| 53 | Yes. I'm doing it right now (after the Labor Day holiday 😄 ) | |
When compiling under Windows with cmake -DPOLLY_ENABLE_GPGPU_CODEGEN=ON, I get a linker error:
Polly.lib(PPCGCodeGeneration.obj) : error LNK2019: unresolved external symbol "class llvm::Pass * __cdecl polly::createDOTOnlyPrinterPass(void)" (?createDOTOnlyPrinterPass@polly@@YAPEAVPass@llvm@@XZ) referenced in function "public: __cdecl `anonymous namespace'::PollyForcePassLinki ng::PollyForcePassLinking(void)" (??0PollyForcePassLinking@?A0x3c23cca7@@QEAA@XZ) [C:\Users\meinersbur\build\llvm-project\debug_vs17\tools\opt\opt.vcxproj] Polly.lib(PPCGCodeGeneration.obj) : error LNK2019: unresolved external symbol "class llvm::Pass * __cdecl polly::createDOTOnlyViewerPass(void)" (?createDOTOnlyViewerPass@polly@@YAPEAVPass@llvm@@XZ) referenced in function "public: __cdecl `anonymous namespace'::PollyForcePassLinking ::PollyForcePassLinking(void)" (??0PollyForcePassLinking@?A0x3c23cca7@@QEAA@XZ) [C:\Users\meinersbur\build\llvm-project\debug_vs17\tools\opt\opt.vcxproj] Polly.lib(PPCGCodeGeneration.obj) : error LNK2019: unresolved external symbol "class llvm::Pass * __cdecl polly::createDOTPrinterPass(void)" (?createDOTPrinterPass@polly@@YAPEAVPass@llvm@@XZ) referenced in function "public: __cdecl `anonymous namespace'::PollyForcePassLinking::Poll yForcePassLinking(void)" (??0PollyForcePassLinking@?A0x3c23cca7@@QEAA@XZ) [C:\Users\meinersbur\build\llvm-project\debug_vs17\tools\opt\opt.vcxproj] Polly.lib(PPCGCodeGeneration.obj) : error LNK2019: unresolved external symbol "class llvm::Pass * __cdecl polly::createDOTViewerPass(void)" (?createDOTViewerPass@polly@@YAPEAVPass@llvm@@XZ) referenced in function "public: __cdecl `anonymous namespace'::PollyForcePassLinking::PollyF orcePassLinking(void)" (??0PollyForcePassLinking@?A0x3c23cca7@@QEAA@XZ) [C:\Users\meinersbur\build\llvm-project\debug_vs17\tools\opt\opt.vcxproj] C:\Users\meinersbur\build\llvm-project\debug_vs17\Debug\bin\opt.exe : fatal error LNK1120: 4 unresolved externals [C:\Users\meinersbur\build\llvm-project\debug_vs17\tools\opt\opt.vcxproj]
I could not yet find a config that also fails under Linux, but it looks like some library dependencies are wrong.
| polly/lib/Analysis/ScopGraphPrinter.cpp | ||
|---|---|---|
| 74–77 | I get a compiler error here: /home/meinersbur/src/llvm-project/polly/lib/Analysis/ScopGraphPrinter.cpp:73:6: error: redeclaration of ‘static void llvm::DOTGraphTraits<polly::ScopDetection>::printRegionCluster(const polly::ScopDetection&, const llvm::Region*, llvm::raw_ostream&, unsigned int)’ may not have default arguments [-fpermissive]
73 | void DOTGraphTraits<ScopDetection>::printRegionCluster(const ScopDetection &SD,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~The compiler is correct here, default parameters belong to the declaration, not definition. The pre-merge check does not seem to have caught this, maybe using a different compiler version (I am using gcc 11.2) | |
Is it a clean build? I guess there are some cache breaks the linking. The function createDOTOnlyViewerPass is removed and replaced by createDOTOnlyViewerWrapperPass in this commit. Check the PollyForcePassLinking, it only calls the createDOTOnlyViewerWrapperPass.
I'm not sure whether this problem shows potential "dependencies issues" (as it seems failed to invalidate the cache) in the build system.
| llvm/include/llvm/Analysis/RegionPrinter.h | ||
|---|---|---|
| 30 | Nothing unusual of a declaration in RegionPrinter.h to be implemented in RegionPrinter.cpp. | |
| polly/include/polly/ScopGraphPrinter.h | ||
| 34 | Looking at other specializations, all GraphTraits of an analysis are a pointer of the the analysis (GraphTraits<ScopDetection>). Why the change? I fear that GraphTraits users may accidentally try to copy the object. | |
| 63 | Consider using llvm::StringRef | |
| 65–66 | Use doxygen comments for API documentation. | |
| polly/lib/Analysis/ScopGraphPrinter.cpp | ||
| 96–97 | Consider DOTGraphTraits<ScopDetection*> | |
| polly/include/polly/ScopGraphPrinter.h | ||
|---|---|---|
| 34 |
Because we could get the reference to ScopDetection from FAM.getResult, and it's quite straightforward to pass the reference to everywhere.
When will it happen? The arguments of the functions are all passing through reference. But as I'm not familiar with GraphTraits and its other usage, I think it's fine not to touch it in this commit. I will use GraphTraits<ScopDetection *> instead. | |
| polly/include/polly/ScopGraphPrinter.h | ||
|---|---|---|
| 28 | @YangKeao Can you remove the 'using namespace' from the header and move them internally to each source file? This might be the cause of the namespace resolution build failures on: https://github.com/compiler-explorer/compiler-workflows/runs/6452927622 | |
| polly/include/polly/ScopGraphPrinter.h | ||
|---|---|---|
| 28 | I was just looking into it when I noticed you already changed it. Thanks you. | |