awarzynski added a comment.
Thanks for trimming this, it's much easier to review! A few more suggestions,
but nothing major.
================
Comment at: flang/lib/Frontend/CompilerInvocation.cpp:227-263
+ bool needLocTracking = false;
+
+ if (!opts.OptRecordFile.empty())
+ needLocTracking = true;
if (const llvm::opt::Arg *a =
+ args.getLastArg(clang::driver::options::OPT_opt_record_passes)) {
----------------
```lang=cpp
// coment
if (const llvm::opt::Arg *a =
args.getLastArg(clang::driver::options::OPT_opt_record_passes))
opts.OptRecordPasses = a->getValue();
// coment
if (const llvm::opt::Arg *a =
args.getLastArg(clang::driver::options::OPT_opt_record_format))
opts.OptRecordFormat = a->getValue();
// coment
opts.OptimizationRemark = parseOptimizationRemark(
diags, args, clang::driver::options::OPT_Rpass_EQ, "pass");
// coment
opts.OptimizationRemarkMissed = parseOptimizationRemark(
diags, args, clang::driver::options::OPT_Rpass_missed_EQ, "pass-missed");
// coment
opts.OptimizationRemarkAnalysis = parseOptimizationRemark(
diags, args, clang::driver::options::OPT_Rpass_analysis_EQ,
"pass-analysis");
if (opts.getDebugInfo() == llvm::codegenoptions::NoDebugInfo) {
// If the user requested a flag that requires source locations available in
// the backend, make sure that the backend tracks source location
information.
bool needLocTracking = !opts.OptRecordFile.empty() || opts.OptRecordPasses
||
!opts.OptRecordFormat.empty() ||
opts.OptimizationRemark.hasValidPattern() ||
opts.OptimizationRemarkMissed.hasValidPattern() ||
opts.OptimizationRemarkAnalysis.hasValidPattern();
if (needLocTracking)
opts.setDebugInfo(llvm::codegenoptions::LocTrackingOnly);
}
```
I might have made typos when editing this, but hopefully the overall logic
makes sense. Basically, I am suggesting that "option parsing" is separated from
the logic for setting up the location tracking in the backend.
================
Comment at: flang/lib/Frontend/FrontendActions.cpp:978-1005
+ if (d.isPassed()) {
+ // Optimization remarks are active only if the -Rpass flag has a regular
+ // expression that matches the name of the pass name in \p D.
+ if (codeGenOpts.OptimizationRemark.patternMatches(d.getPassName()))
+ emitOptimizationMessage(
+ d, clang::diag::remark_fe_backend_optimization_remark);
+
----------------
https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code
```lang=cpp
void
optimizationRemarkHandler(const llvm::DiagnosticInfoOptimizationBase &d) {
if (d.isPassed()) {
// Optimization remarks are active only if the -Rpass flag has a regular
// expression that matches the name of the pass name in \p D.
if (codeGenOpts.OptimizationRemark.patternMatches(d.getPassName()))
emitOptimizationMessage(
d, clang::diag::remark_fe_backend_optimization_remark);
return;
}
if (d.isMissed()) {
// Missed optimization remarks are active only if the -Rpass-missed
// flag has a regular expression that matches the name of the pass
// name in \p D.
if (codeGenOpts.OptimizationRemarkMissed.patternMatches(d.getPassName()))
emitOptimizationMessage(
d, clang::diag::remark_fe_backend_optimization_remark_missed);
return;
}
assert(d.isAnalysis() && "Unknown remark type");
bool shouldAlwaysPrint = false;
if (auto *ora = llvm::dyn_cast<llvm::OptimizationRemarkAnalysis>(&d))
shouldAlwaysPrint = ora->shouldAlwaysPrint();
if (shouldAlwaysPrint ||
codeGenOpts.OptimizationRemarkAnalysis.patternMatches(
d.getPassName()))
emitOptimizationMessage(
d, clang::diag::remark_fe_backend_optimization_remark_analysis);
}
```
================
Comment at: flang/lib/Frontend/FrontendActions.cpp:1007-1024
+ bool handleDiagnostics(const llvm::DiagnosticInfo &di) override {
+ switch (di.getKind()) {
+ case llvm::DK_OptimizationRemark:
+ optimizationRemarkHandler(llvm::cast<llvm::OptimizationRemark>(di));
+ break;
+ case llvm::DK_OptimizationRemarkMissed:
+
optimizationRemarkHandler(llvm::cast<llvm::OptimizationRemarkMissed>(di));
----------------
Where is this method used?
================
Comment at: flang/lib/Frontend/FrontendActions.cpp:1136
+ BackendRemarkConsumer m(diags, codeGenOpts);
+
----------------
https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
> Variable names should be nouns (as they represent state). The name should be
> camel case, and start with an upper case letter (e.g. Leader or Boats).
================
Comment at: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:169-171
+ clang::ProcessWarningOptions(flang->getDiagnostics(),
+ flang->getDiagnosticOpts());
+
----------------
Is this calling
https://github.com/llvm/llvm-project/blob/c52d9509d40d3048914b144618232213e6076e05/clang/include/clang/Basic/Diagnostic.h#L1840-L1842?
That's part of the `clangBasic` library. The overall goal in the driver is to
reduce the dependency on Clang and this would be a step in the opposite
direction. IIUC, we only need a small subset of that function, right?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156320/new/
https://reviews.llvm.org/D156320
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
