Migration to the new pass manager in optimization routines #4
Conversation
Focshole
force-pushed
the
renovation-work
branch
from
bb78548
to
3c04add
Compare
Focshole
changed the title
Focshole
force-pushed
the
renovation-work
branch
2 times, most recently
from
ad40186
to
fc5de1e
Compare
Focshole
force-pushed
the
renovation-work
branch
from
fc5de1e
to
f8041dd
Compare
| #ifndef LIB_VERSIONING_COMPILER_CLANG_LLVM_TOOLS_OPT_NEWPMDRIVER_H | ||
| #define LIB_VERSIONING_COMPILER_CLANG_LLVM_TOOLS_OPT_NEWPMDRIVER_H |
There was a problem hiding this comment.
Shouldn't this file be a HPP file instead of a H file?
There was a problem hiding this comment.
is there a chain of includes that requires this file to stay .h?
There was a problem hiding this comment.
I kept it identical to the original file name, so that it can be easily referenced.
https://github.com/llvm/llvm-project/blob/main/llvm/tools/opt/NewPMDriver.h
There was a problem hiding this comment.
Ok, makes sense.
However, please add a comment right near the top to specify this
| vc::Option("mem2reg", "-passes='defaultO3,mem2reg'"), | ||
| vc::Option("optimization passes", "-passes=","'default<O3>,mem2reg'"), |
There was a problem hiding this comment.
this way of writing pass names or pass sets is not of immediate understanding, can you add a comment to mention where default<O3> is coming from, for example?
There was a problem hiding this comment.
Yes, sadly on LLVM they chose this kind of syntax. See https://github.com/llvm/llvm-project/blob/65dca4cbcbfc5c4447207eff7d223005089e9fed/llvm/tools/opt/opt.cpp#L177
There was a problem hiding this comment.
Can you find an appropriate point in this file to add a reference to the opt command option guide?
| @@ -107,7 +107,7 @@ int main(int argc, char const *argv[]) { | |||
| }); | |||
| #else | |||
| builder.setOptOptions({ | |||
| vc::Option("mem2reg", "-passes='defaultO3,mem2reg'"), | |||
| vc::Option("optimization passes", "-passes=","'default<O3>,mem2reg'"), | |||
There was a problem hiding this comment.
this way of writing pass names or pass sets is not of immediate understanding, can you add a comment to mention where default<O3> is coming from, for example?
There was a problem hiding this comment.
Same as above, ideally there should be a reference in each test/example file.
include/versioningCompiler/CompilerImpl/ClangLLVM/LLVMInstanceManager.hpp
Outdated
Show resolved
Hide resolved
lib/CompilerImpl/JITCompiler.cpp
Outdated
| // #ifdef VC_DEBUG | ||
| // Before executing passes, print the final values of the LLVM options. | ||
| llvm::cl::PrintOptionValues(); | ||
| // #endif |
There was a problem hiding this comment.
why is this no longer only enabled in debug mode?
Co-authored-by: Stefano Cherubin <skeru@users.noreply.github.com>
…Manager.hpp Co-authored-by: Stefano Cherubin <skeru@users.noreply.github.com>
Co-authored-by: Stefano Cherubin <skeru@users.noreply.github.com>
Co-authored-by: Stefano Cherubin <skeru@users.noreply.github.com>
Co-authored-by: Stefano Cherubin <skeru@users.noreply.github.com>
Co-authored-by: Stefano Cherubin <skeru@users.noreply.github.com>
Co-authored-by: Stefano Cherubin <skeru@users.noreply.github.com>
It is based on the previous work to port to LLVM15. It finally does the migration to the new pass manager in a maintainable way (edits from original LLVM files are highlighted). It is thought for LLVM 16