[PATCH] D114122: [CMake] Add new cmake option to control adding comments in GenDAGISel

Thu Nov 18 18:14:29 PST 2021

MaskRay added subscribers: dmgreen, RKSimon, craig.topper, arsenm.
MaskRay added a comment.

In D114122#3139717 <https://reviews.llvm.org/D114122#3139717>, @nemanjai wrote:

> In D114122#3139130 <https://reviews.llvm.org/D114122#3139130>, @MaskRay wrote:
>
>> Add a new CMake variable similar to `LLVM_OPTIMIZED_TABLEGEN`? I think most people don't need comments even for their `!NDEBUG` builds.
>
> I always wonder about statements about what "most people need/want". From my experience, most people in my immediate surroundings actually do use debug messages from ISEL and want them to refer to something they can find in `.inc` files rather than being meaningless. Most of those same people simply don't build debug builds because those are very slow, huge, etc. while just asserts provide enough "debuggability".
>
> While having messages that refer to indices in a file that do not exist seems like simply a bad idea (i.e. why emit meaningless messages at all), I do recognize the fact that comments in these `.inc` files slow everyone's build and are only useful to back end developers.
>
> All that being said, I am not against adding a CMake variable (perhaps `LLVM_OMIT_DAGISEL_COMMENTS`) that will be set to `true` for `Release` builds and `false` for `Debug/RelWithDebInfo` builds.

I know that you mentioned it. "most people need/want" is anecdotal and from my observation that no other backend developer has complained about it. I could have missed something, though.

To be productive, perhaps we can tag some backend developers. @RKSimon @craig.topper @arsenm @dmgreen

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114122