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

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added a comment.
Herald added a subscriber: wuzish.

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",


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78884