victorkingi added a comment. In D156320#4594584 <https://reviews.llvm.org/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 traffic/noise and use more descriptive summaries. > > Also, rather than adding new features in this already large change (I am > referring to e.g. `DK_MachineOptimizationRemarkMissed`), please try to > identify ways to split this patch further. Here are some suggestions (I've > also made comments inline): > > 1. In the first iteration (like you effectively do now), focus on > OPT_R_Joined > <https://github.com/llvm/llvm-project/blob/c52d9509d40d3048914b144618232213e6076e05/clang/include/clang/Driver/Options.td#L2893-L2894> > options (e.g. `-Rpass`, `Rpass-analysis`, `-Rpass-missed`). Focus on basic > functionality that demonstrates that correct information is returned from the > backend. No need to fine tune the remarks with e.g. full file path or > relevant remark option. > 2. Next, add support for -Rpass=/-Rpass-missed=/-Rpass-analysis= > <https://github.com/llvm/llvm-project/blob/c52d9509d40d3048914b144618232213e6076e05/clang/include/clang/Driver/Options.td#L2884-L2892>. > That's when e.g. `llvm::opt::OptSpecifier optEq` in > `parseOptimizationRemark` would be needed (but not in Step 1). > 3. Fine tune how the report is printed (e.g. improve file name by adding full > path, add valid remark option at the end etc). > 4. Add support for machine optimisations, e.g. > `DK_MachineOptimizationRemarkMissed`. > > This is easily 4 patches ;-) Hi Andrzej, splitting into 4 patches seems like a good idea. Here's the first patch that only handles the backend implementation https://reviews.llvm.org/D158174 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
