[PATCH] D78884: [CMake] -gen-dag-isel: add -omit-comments if neither Debug nor RelWithDebInfo

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D78884#2081075 <https://reviews.llvm.org/D78884#2081075>, @nemanjai wrote:

> I am very much opposed to this. I just wish I noticed it before it was approved and pushed. In my opinion, the debug dumps from the SDAG are the most useful and easiest to follow of all of the passes in LLVM. A release + asserts build is a very common way for people to build and without the comments in the `.inc` files, the debug dumps from the SDAG are useless.
>
> I understand that this reduces the size of the produced code, but I don't think the cost is worth the benefit. Personally, I would not differentiate this from other things we get under `#ifndef NDEBUG`. If you don't want the comments, build with something that defines `NDEBUG`.
>  So I think a much better way to disable comments would be something like:
>
>   diff --git a/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp b/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp
>   index d9ec14a..99bc3e9 100644
>   --- a/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp
>   +++ b/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp
>   @@ -36,7 +36,12 @@ cl::OptionCategory DAGISelCat("Options for -gen-dag-isel");
>    // To reduce generated source code size.
>    static cl::opt<bool> OmitComments("omit-comments",
>                                      cl::desc("Do not generate comments"),
>   -                                  cl::init(false), cl::cat(DAGISelCat));
>   +#ifndef NDEBUG
>   +                                  cl::init(false),
>   +#else
>   +                                  cl::init(true),
>   +#endif
>   +                                  cl::cat(DAGISelCat));
>   
>    static cl::opt<bool> InstrumentCoverage(
>        "instrument-coverage",
>


How are comments helpful if no debugging information is produced (neither RelWithDebInfo nor Debug)? You don't have line tables and the debugger cannot associate the source lines. If you have reasons that `-UNDEBUG` builds may need comments, we can check LLVM_ENABLE_ASSERTIONS in CMake as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78884/new/

https://reviews.llvm.org/D78884