Page MenuHomePhabricator

[IR] Use LLVM_ENABLE_ABI_BREAKING_CHECKS to guard ABI changes.
ClosedPublic

Authored by varungandhi-apple on Dec 16 2020, 5:10 PM.

Details

Summary

Incorrect usage of NDEBUG to guard ABI changes can prevent clients
from enabling assertions for their C++ code while having assertions in
LLVM turned off. So we use LLVM_ENABLE_ABI_BREAKING_CHECKS instead, as
described in llvm/docs/ProgrammersManual.rst. Most types already use this
macro, however, there were a couple of stragglers in ValueHandle.h, which
are fixed by this revision.

Diff Detail

Event Timeline

varungandhi-apple requested review of this revision.Dec 16 2020, 5:10 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 16 2020, 5:10 PM
dblaikie accepted this revision.Dec 16 2020, 5:19 PM

Looks good to me - couple of side comments, possibly more directed at other reviewers to discuss/consider.

llvm/include/llvm/IR/ValueHandle.h
290

Follow-up/preliminary commit could probably remove the <ValueTy> from this line, I think.

447

Would it be OK (follow-up/preliminary commit) to move final out of this block? So the type is always final - or does that break something else?

This revision is now accepted and ready to land.Dec 16 2020, 5:19 PM
dexonsmith added inline comments.Fri, Dec 18, 8:04 PM
llvm/include/llvm/IR/ValueHandle.h
447

Interesting; I hadn't considered putting final on a class that doesn't inherit from anything, but I imagine you're right and this could be moved out.

dexonsmith accepted this revision.Fri, Dec 18, 8:05 PM

(LGTM too)

Going to land this as-is, will make suggested tweaks in a follow-up change.

dblaikie added inline comments.Thu, Jan 7, 2:19 PM
llvm/include/llvm/IR/ValueHandle.h
489

Missed this usage (probably worth building with/without ABI breaking checks in the future to validate these sort of changes) which breaks the non-abi-breaking-checks build. Oh, and some death unit tests no longer worked/needed to be protected by ABI_BREAKING_CHECKS as well as !NDEBUG.

Fixed in: 3503c856819efc01ce210fa56e597ddfb7a4c1a1