morehouse added inline comments.
================ Comment at: clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt:17 -# Depend on LLVM IR intrinsic generation. +# Depend on LLVM IR instrinsic generation. set(handle_llvm_deps intrinsics_gen) ---------------- Typo introduced here. ================ Comment at: clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt:25 handle_llvm.cpp - + DEPENDS ---------------- Please don't add whitespace to a previously empty line. ================ Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:10 // -// Implements HandleLLVM for use by the Clang fuzzers. Mimics the llc tool to -// compile an LLVM IR file to X86_64 assembly. +// Implements HandleLLVM for use by the Clang fuzzers. First runs an loop +// vectorizer optimization pass over the given IR code. Then mimics lli on both ---------------- an -> a ================ Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:112 +// Mimics the opt tool to run an optimization pass over the provided IR +std::string OptLLVM(const std::string IR, CodeGenOpt::Level &OLvl) { + InitEverything(); ---------------- Maybe this should be `const std::string &IR` or `StringRef`. ================ Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:113 +std::string OptLLVM(const std::string IR, CodeGenOpt::Level &OLvl) { + InitEverything(); + ---------------- Does this need to be called every time we get a new input? Can we call this from `LLVMFuzzerInitialize()`? ================ Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:123 + args[1] = arg1.c_str(); + cl::ParseCommandLineOptions(2, args); ---------------- This shouldn't be required. You should be able to directly add the passes you want. See `llvm/lib/Passes/PassBuilder.cpp`. ================ Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:132 + + Triple ModuleTriple(M->getTargetTriple()); + TargetOptions Options = InitTargetOptionsFromCodeGenFlags(); ---------------- Move this closer to where it's used. ================ Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:133 + Triple ModuleTriple(M->getTargetTriple()); + TargetOptions Options = InitTargetOptionsFromCodeGenFlags(); + std::string CPUStr; ---------------- `Options` is never used. ================ Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:136 + std::string FeaturesStr; + std::unique_ptr<TargetMachine> TM(nullptr); + setFunctionAttributes(CPUStr, FeaturesStr, *M); ---------------- `TM` is never used. ================ Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:137 + std::unique_ptr<TargetMachine> TM(nullptr); + setFunctionAttributes(CPUStr, FeaturesStr, *M); + ---------------- Does this do anything since `CPUStr` and `FeaturesStr` are empty? ================ Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:145 + std::unique_ptr<legacy::FunctionPassManager> FPasses; + FPasses.reset(new legacy::FunctionPassManager(M.get())); + FPasses->add(createTargetTransformInfoWrapperPass(TargetIRAnalysis())); ---------------- Can we just make `FPasses` on stack instead? ================ Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:190 + builder.setMCJITMemoryManager( + std::unique_ptr<RTDyldMemoryManager>(RTDyldMM)); + builder.setOptLevel(OLvl); ---------------- These 3 lines can be combined to `builder.setMCJITMemoryManager(new SectionMemoryManager())` 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