[PATCH] D120714: [CodeGen] Use SDNode debug ID declaration in release builds

[Daniil Kovalev via Phabricator via llvm-commits]

kovdan01 added a comment.

@dexonsmith

In D120714#3400246 <https://reviews.llvm.org/D120714#3400246>, @dexonsmith wrote:

> Thanks for the pings! I'd missed your updates.
>
> In D120714#3377826 <https://reviews.llvm.org/D120714#3377826>, @kovdan01 wrote:
>
>> @dexonsmith Thanks for your comments! Decided to choose the last approach that you mentioned - keep and use `PersistentId` regardless build type or any macro. The problem with `#if LLVM_ENABLE_ABI_BREAKING_CHECKS` is testing with `LLVM_ABI_BREAKING_CHECKS` set to `FORCE_OFF` during initial CMake configuration - some tests rely on PersistentId. I suppose that having PersistentId present in all builds is definitely worth small CPU overhead compared to the current state.
>
> Which tests are you talking about? Can those tests be marked as UNSUPPORTED when ABI-breaking checks are off?
>
> If not, why not?
>
> It would be good to add a comment to the sources explaining why `LLVM_ENABLE_ABI_BREAKING_CHECKS` can't/shouldn't be used for `PersistentId`, and potentially a FIXME with how to fix the blockers such as they are. Probably next to the declaration of the field.

Currently there is no check for ABI-breaking changes (see llvm/test/lit.site.cfg.py.in). Of course, that could be easily added, but I decided to update the tests themselves to make them work with SDNode debug IDs both in t<number> and 0x<hex address> formats. Also changes update_llc_test_checks.py, and it looks like that we can support more test scenarios for isel in it (not only resulting selection DAG, but also initial and intermediate ones, instcombine phase, etc.)


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

https://reviews.llvm.org/D120714