[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi abandoned this revision. victorkingi added a comment. Replaced by a series of patches already merged on github, starting from https://reviews.llvm.org/D157410 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156320/new/

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi added a comment. In D156320#4594584 , @awarzynski wrote: > Victor, this is proving quite tricky to review. There's already been a lot of > updates and many of them are summarized as either "code refactor" or > "clean-up". Please reduce

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

awarzynski added a comment. Victor, this is proving quite tricky to review. There's already been a lot of updates and many of them are summarized as either "code refactor" or "clean-up". Please reduce traffic/noise and use more descriptive summaries. Also, rather than adding new features in

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

tschuett added a comment. Thanks! 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

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi added inline comments. Comment at: flang/lib/Frontend/FrontendActions.cpp:1032-1043 +case llvm::DK_MachineOptimizationRemark: + optimizationRemarkHandler( + llvm::cast(di)); + break; +case llvm::DK_MachineOptimizationRemarkMissed: +

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi updated this revision to Diff 550800. victorkingi added a comment. added support for backend remarks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156320/new/ https://reviews.llvm.org/D156320 Files:

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi added inline comments. Comment at: flang/test/Driver/optimization-remark.f90:7 +! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS +! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

tschuett added inline comments. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:227 + // Specifies, using a regex, which successful optimization passes done, + // to include in the final optimization record file generated. If not provided victorkingi

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi added inline comments. Comment at: flang/include/flang/Frontend/CodeGenOptions.h:76-81 +RK_Missing,// Remark argument not present on the command line. +RK_Enabled,// Remark enabled via '-Rgroup'. +RK_EnabledEverything, // Remark

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi updated this revision to Diff 550725. victorkingi added a comment. code refactoring Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156320/new/ https://reviews.llvm.org/D156320 Files: flang/include/flang/Frontend/CodeGenOptions.h

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi updated this revision to Diff 550719. victorkingi marked 9 inline comments as done. victorkingi added a comment. code refactoring Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156320/new/ https://reviews.llvm.org/D156320 Files:

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

tschuett added inline comments. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:227 + // Specifies, using a regex, which successful optimization passes done, + // to include in the final optimization record file generated. If not provided awarzynski

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

awarzynski added a comment. hey @victorkingi , I am still unsure about parsing these remarks options in two places: - CompilerInvocation.cpp - ExecuteCompilerInvocation.cpp I think that it is important to clarify the relations between the two. In particular, it's normally the job of

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi updated this revision to Diff 550345. victorkingi added a comment. rebasing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156320/new/ https://reviews.llvm.org/D156320 Files: flang/include/flang/Frontend/CodeGenOptions.h

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi updated this revision to Diff 550320. victorkingi added a comment. rebasing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156320/new/ https://reviews.llvm.org/D156320 Files: flang/include/flang/Frontend/CodeGenOptions.h

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi added inline comments. Comment at: flang/lib/Frontend/FrontendActions.cpp:976-1011 + void + optimizationRemarkHandler(const llvm::DiagnosticInfoOptimizationBase ) { +if (d.isPassed()) { + // Optimization remarks are active only if the -Rpass flag has a

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi updated this revision to Diff 550288. victorkingi added a comment. code refactoring Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156320/new/ https://reviews.llvm.org/D156320 Files: flang/include/flang/Frontend/CodeGenOptions.h

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi updated this revision to Diff 550280. victorkingi marked 4 inline comments as done. victorkingi added a comment. code refactoring Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156320/new/ https://reviews.llvm.org/D156320 Files:

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi updated this revision to Diff 550232. victorkingi marked 5 inline comments as done. victorkingi added a comment. code refactoring Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156320/new/ https://reviews.llvm.org/D156320 Files:

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

awarzynski added a comment. Some high level comments: - The logic in `parseCodeGenArgs` in CompilerInvocation.cpp is a bit complex and quite specialized - could you move it to a dedicated method? - In a fair few places this patch make references to "diagnostic flags" or "diagnostic options".

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

awarzynski added a comment. Thanks for the updates - this is looking really good! A few more suggestions and then I'll scan the whole thing again (sorry, there's been quite a lot of code going back and forth). Comment at: flang/lib/Frontend/CompilerInvocation.cpp:1024 +

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi marked 6 inline comments as done. victorkingi added inline comments. Comment at: flang/lib/Frontend/FrontendActions.cpp:927 +class StandaloneBackendConsumer : public llvm::DiagnosticHandler { + awarzynski wrote: > Why `StandaloneBackendConsumer`?

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi added inline comments. Comment at: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:169-171 + clang::ProcessWarningOptions(flang->getDiagnostics(), + flang->getDiagnosticOpts()); + victorkingi wrote: > awarzynski

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi added inline comments. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:156-158 +/// Parse a remark command line argument. It may be missing, disabled/enabled by +/// '-R[no-]group' or specified with a regular expression by '-Rgroup=regexp'. +/// On top of

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi updated this revision to Diff 549887. victorkingi added a comment. added comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156320/new/ https://reviews.llvm.org/D156320 Files: flang/include/flang/Frontend/CodeGenOptions.h

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi updated this revision to Diff 549882. victorkingi added a comment. added tests for "no" variants of Rpass Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156320/new/ https://reviews.llvm.org/D156320 Files:

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi updated this revision to Diff 549879. victorkingi marked 6 inline comments as done. victorkingi added a comment. code refactoring in reference to comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156320/new/

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

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()) +

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi updated this revision to Diff 549367. victorkingi marked an inline comment as done. victorkingi added a comment. addressing comments with code refactoring Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156320/new/

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

awarzynski added inline comments. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:156-158 +/// Parse a remark command line argument. It may be missing, disabled/enabled by +/// '-R[no-]group' or specified with a regular expression by '-Rgroup=regexp'. +/// On top of that,

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

kiranchandramohan added inline comments. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:156-158 +/// Parse a remark command line argument. It may be missing, disabled/enabled by +/// '-R[no-]group' or specified with a regular expression by '-Rgroup=regexp'. +/// On top

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

awarzynski added a comment. Thanks for the updates, more comments inline. Also: > The remarks printed miss carets. Why and can you share an example? > The parseOptimizationRemark, addDiagnosticArgs and printDiagnosticOptions > functions created are identical to the ones used in Clang. In

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi updated this revision to Diff 549000. victorkingi added a comment. Added warning tests in optimization-remark.f90 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156320/new/ https://reviews.llvm.org/D156320 Files:

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi updated this revision to Diff 548950. victorkingi added a comment. Removed false argument to ProcessWarningOption function to allow warning printing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156320/new/

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi updated this revision to Diff 548941. victorkingi added a comment. rebasing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156320/new/ https://reviews.llvm.org/D156320 Files: flang/include/flang/Frontend/CodeGenOptions.h

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi marked an inline comment as done. victorkingi added inline comments. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:786 parseShowColorsArgs(args, /*defaultDiagColor=*/false); + res.getDiagnosticOpts().ShowColors = res.getFrontendOpts().showColors;

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

victorkingi updated this revision to Diff 548664. victorkingi edited the summary of this revision. victorkingi added a comment. Added remark error test and color printing for remark errors Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156320/new/

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

awarzynski added inline comments. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:173-174 +if (!result.Regex->isValid(regexError)) { + diags.Report(clang::diag::err_drv_optimization_remark_pattern) + << regexError << a->getAsString(args); + return