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

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

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.


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

https://reviews.llvm.org/D120714