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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D114122#3141721 <https://reviews.llvm.org/D114122#3141721>, @MaskRay wrote:

> 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

These days I don't find myself using the numbers in those debug message very often. If I do need the comments in the GenDAGISel.inc, I run llvm-tblgen manually to generate a copy with the comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114122