morehouse added inline comments.
================
Comment at: clang/tools/clang-fuzzer/fuzzer-initialize/fuzzer_initialize.cpp:44
+
+ PassRegistry &Registry = *llvm::PassRegistry::getPassRegistry();
+ initializeCore(Registry);
----------------
Unnecessary `llvm::`
================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:89
+ Builder.Inliner = createFunctionInliningPass(OptLevel, SizeLevel, false);
+ Builder.LoopVectorize = true;
+ Builder.populateFunctionPassManager(FPM);
----------------
If we do this, do we need to explicitly add the vectorizer pass below?
================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:95
+// Mimics the opt tool to run an optimization pass over the provided IR
+std::string OptLLVM(const std::string &IR, CodeGenOpt::Level &OLvl) {
+ // Create a module that will run the optimization passes
----------------
`OLvl` is never used.
================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:105
+ std::string FeaturesStr = getFeaturesStr();
+ setFunctionAttributes(CPUStr, FeaturesStr, *M);
+
----------------
Let's simplify to `setFunctionAttributes(getCPUStr(), getFeaturesStr(), *M)`.
================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:110
+ TargetLibraryInfoImpl TLII(ModuleTriple);
+ Passes.add(new TargetLibraryInfoWrapperPass(TLII));
+ Passes.add(createTargetTransformInfoWrapperPass(TargetIRAnalysis()));
----------------
Can simplify to `Passes.add(new
TargetLibraryInfoWrapperPass(M->getTargetTriple))`.
================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:115
+ make_unique<legacy::FunctionPassManager>(M.get());
+ FPasses->add(createTargetTransformInfoWrapperPass(TargetIRAnalysis()));
+
----------------
Why do we add a `TargetTransformInfoWrapperPass` to both `Passes` and `FPasses`?
================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:115
+ make_unique<legacy::FunctionPassManager>(M.get());
+ FPasses->add(createTargetTransformInfoWrapperPass(TargetIRAnalysis()));
+
----------------
morehouse wrote:
> Why do we add a `TargetTransformInfoWrapperPass` to both `Passes` and
> `FPasses`?
Do we need both `Passes` and `FPasses`?
================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:121
+ ErrorAndExit("Cannot create IR pass");
+ Passes.add(P);
+
----------------
Let's simplify to `Passes.add(createLoopVectorizePass())`.
================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:127
+ FPasses->doFinalization();
+ Passes.add(createVerifierPass());
+
----------------
Why do we keep adding passes in different places?
================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:144
+ Context);
+ Module *M = Owner.get();
+ if (!M)
----------------
Why not just rename `Owner` to `M` and remove this line?
================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:158
+ builder.setMCJITMemoryManager(
+ std::unique_ptr<RTDyldMemoryManager>(RTDyldMM));
+ builder.setOptLevel(OLvl);
----------------
Please simplify to
`builder.setMCJITMemoryManager(std::make_unique<RTDyldMemoryManager>())`.
================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:159
+ std::unique_ptr<RTDyldMemoryManager>(RTDyldMM));
+ builder.setOptLevel(OLvl);
+ builder.setTargetOptions(InitTargetOptionsFromCodeGenFlags());
----------------
If the JIT does optimization already, do we need to call `OptLLVM` at all?
Will the vectorizer kick in without `OptLLVM`?
================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:168
+ if (!EntryFunc)
+ ErrorAndExit("Function not found in module");
----------------
Move this closer to where it's used.
================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:171
+ EE->finalizeObject();
+ EE->runStaticConstructorsDestructors(false);
+
----------------
Do we need to call this again to run destructors after we execute `f()` ?
================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:181
- raw_null_ostream OS;
- Target->addPassesToEmitFile(PM, OS, nullptr, TargetMachine::CGFT_ObjectFile,
- false);
- PM.run(*M);
+ (*f)(a, b, c, 1);
+}
----------------
I think `f(a, b, c, 1)` will also work.
Repository:
rC Clang
https://reviews.llvm.org/D49526
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
