Skip to content

Commit

Permalink
Revert "Fix LLVM_ENABLE_ABI_BREAKING_CHECKS macro check: use #if inst… (
Browse files Browse the repository at this point in the history
llvm#110923)

…ead of #ifdef (llvm#110883)"

This reverts commit 1905cdb, which
causes lots of failures where LLVM doesn't have the right header guards.
The errors can be seen on
[BuildKite](https://buildkite.com/llvm-project/upstream-bazel/builds/112362#01924eae-231c-4d06-ba87-2c538cf40e04),
where the source uses `#ifndef NDEBUG`, but the content in question is
defined when `LLVM_ENABLE_ABI_BREAKING_CHECKS == 1`.

For example, `llvm/include/llvm/Support/GenericDomTreeConstruction.h`
has the following:

```cpp
// Helper struct used during edge insertions.
struct InsertionInfo {
  // ...
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
  SmallVector<TreeNodePtr, 8> VisitedUnaffected;
#endif
};

// ...

InsertionInfo II;
// ...
#ifndef NDEBUG
            II.VisitedUnaffected.push_back(SuccTN);
#endif
```
  • Loading branch information
cjdb authored Oct 2, 2024
1 parent 916e6ad commit 45ad1ac
Show file tree
Hide file tree
Showing 13 changed files with 32 additions and 32 deletions.
2 changes: 1 addition & 1 deletion llvm/include/llvm/CodeGen/SelectionDAG.h
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ class SelectionDAG {
return Root;
}

#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifndef NDEBUG
void VerifyDAGDivergence();
#endif

Expand Down
2 changes: 1 addition & 1 deletion llvm/include/llvm/Passes/StandardInstrumentations.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ class PreservedCFGCheckerInstrumentation {
FunctionAnalysisManager::Invalidator &);
};

#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
SmallVector<StringRef, 8> PassStack;
#endif

Expand Down
4 changes: 2 additions & 2 deletions llvm/include/llvm/Support/GenericDomTreeConstruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,7 @@ struct SemiNCAInfo {
Bucket;
SmallDenseSet<TreeNodePtr, 8> Visited;
SmallVector<TreeNodePtr, 8> Affected;
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
SmallVector<TreeNodePtr, 8> VisitedUnaffected;
#endif
};
Expand Down Expand Up @@ -915,7 +915,7 @@ struct SemiNCAInfo {
LLVM_DEBUG(dbgs() << "Deleting edge " << BlockNamePrinter(From) << " -> "
<< BlockNamePrinter(To) << "\n");

#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
// Ensure that the edge was in fact deleted from the CFG before informing
// the DomTree about it.
// The check is O(N), so run it only in debug configuration.
Expand Down
4 changes: 2 additions & 2 deletions llvm/include/llvm/Transforms/Scalar/JumpThreading.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ class JumpThreadingPass : public PassInfoMixin<JumpThreadingPass> {
bool ChangedSinceLastAnalysisUpdate = false;
bool HasGuards = false;
#ifndef LLVM_ENABLE_ABI_BREAKING_CHECKS
SmallSet<AssertingVH<const BasicBlock>, 16> LoopHeaders;
#else
SmallPtrSet<const BasicBlock *, 16> LoopHeaders;
#else
SmallSet<AssertingVH<const BasicBlock>, 16> LoopHeaders;
#endif

unsigned BBDupThreshold;
Expand Down
6 changes: 3 additions & 3 deletions llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ class LPMUpdater {
}

void setParentLoop(Loop *L) {
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
ParentL = L;
#endif
}
Expand Down Expand Up @@ -295,7 +295,7 @@ class LPMUpdater {
/// loops within them will be visited in postorder as usual for the loop pass
/// manager.
void addSiblingLoops(ArrayRef<Loop *> NewSibLoops) {
#if LLVM_ENABLE_ABI_BREAKING_CHECKS && !defined(NDEBUG)
#if defined(LLVM_ENABLE_ABI_BREAKING_CHECKS) && !defined(NDEBUG)
for (Loop *NewL : NewSibLoops)
assert(NewL->getParentLoop() == ParentL &&
"All of the new loops must be siblings of the current loop!");
Expand Down Expand Up @@ -347,7 +347,7 @@ class LPMUpdater {
const bool LoopNestMode;
bool LoopNestChanged;

#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
// In debug builds we also track the parent loop to implement asserts even in
// the face of loop deletion.
Loop *ParentL;
Expand Down
6 changes: 3 additions & 3 deletions llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
/// consistent when instructions are moved.
SmallVector<SCEVInsertPointGuard *, 8> InsertPointGuards;

#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
const char *DebugType;
#endif

Expand All @@ -184,7 +184,7 @@ class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
Builder(se.getContext(), InstSimplifyFolder(DL),
IRBuilderCallbackInserter(
[this](Instruction *I) { rememberInstruction(I); })) {
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
DebugType = "";
#endif
}
Expand All @@ -194,7 +194,7 @@ class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
assert(InsertPointGuards.empty());
}

#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
void setDebugType(const char *s) { DebugType = s; }
#endif

Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11725,7 +11725,7 @@ void SelectionDAG::CreateTopologicalOrder(std::vector<SDNode *> &Order) {
}
}

#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifndef NDEBUG
void SelectionDAG::VerifyDAGDivergence() {
std::vector<SDNode *> TopoOrder;
CreateTopologicalOrder(TopoOrder);
Expand Down
6 changes: 3 additions & 3 deletions llvm/lib/Passes/StandardInstrumentations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1358,7 +1358,7 @@ void PreservedCFGCheckerInstrumentation::registerCallbacks(
bool Registered = false;
PIC.registerBeforeNonSkippedPassCallback([this, &MAM, Registered](
StringRef P, Any IR) mutable {
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
assert(&PassStack.emplace_back(P));
#endif
(void)this;
Expand Down Expand Up @@ -1387,7 +1387,7 @@ void PreservedCFGCheckerInstrumentation::registerCallbacks(

PIC.registerAfterPassInvalidatedCallback(
[this](StringRef P, const PreservedAnalyses &PassPA) {
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
assert(PassStack.pop_back_val() == P &&
"Before and After callbacks must correspond");
#endif
Expand All @@ -1396,7 +1396,7 @@ void PreservedCFGCheckerInstrumentation::registerCallbacks(

PIC.registerAfterPassCallback([this, &MAM](StringRef P, Any IR,
const PreservedAnalyses &PassPA) {
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
assert(PassStack.pop_back_val() == P &&
"Before and After callbacks must correspond");
#endif
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
#include "llvm/Support/raw_ostream.h"
#include "llvm/Transforms/Utils/LoopUtils.h"

#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
#define SCEV_DEBUG_WITH_TYPE(TYPE, X) DEBUG_WITH_TYPE(TYPE, X)
#else
#define SCEV_DEBUG_WITH_TYPE(TYPE, X)
Expand Down
6 changes: 3 additions & 3 deletions mlir/include/mlir/Analysis/Presburger/PresburgerSpace.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class Identifier {
template <typename T>
explicit Identifier(T value)
: value(llvm::PointerLikeTypeTraits<T>::getAsVoidPointer(value)) {
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
idType = llvm::getTypeName<T>();
#endif
}
Expand All @@ -84,7 +84,7 @@ class Identifier {
/// the type of the identifier used to create it.
template <typename T>
T getValue() const {
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
assert(llvm::getTypeName<T>() == idType &&
"Identifier was initialized with a different type than the one used "
"to retrieve it.");
Expand All @@ -108,7 +108,7 @@ class Identifier {
/// The value of the identifier.
void *value = nullptr;

#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
/// TypeID of the identifiers in space. This should be used in asserts only.
llvm::StringRef idType;
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ class TransformState {
/// should be emitted when the value is used.
using InvalidatedHandleMap = DenseMap<Value, std::function<void(Location)>>;

#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
/// Debug only: A timestamp is associated with each transform IR value, so
/// that invalid iterator usage can be detected more reliably.
using TransformIRTimestampMapping = DenseMap<Value, int64_t>;
Expand All @@ -211,7 +211,7 @@ class TransformState {
ValueMapping values;
ValueMapping reverseValues;

#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
TransformIRTimestampMapping timestamps;
void incrementTimestamp(Value value) { ++timestamps[value]; }
#endif // LLVM_ENABLE_ABI_BREAKING_CHECKS
Expand Down Expand Up @@ -248,7 +248,7 @@ class TransformState {
auto getPayloadOps(Value value) const {
ArrayRef<Operation *> view = getPayloadOpsView(value);

#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
// Memorize the current timestamp and make sure that it has not changed
// when incrementing or dereferencing the iterator returned by this
// function. The timestamp is incremented when the "direct" mapping is
Expand All @@ -259,7 +259,7 @@ class TransformState {
// When ops are replaced/erased, they are replaced with nullptr (until
// the data structure is compacted). Do not enumerate these ops.
return llvm::make_filter_range(view, [=](Operation *op) {
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
[[maybe_unused]] bool sameTimestamp =
currentTimestamp == this->getMapping(value).timestamps.lookup(value);
assert(sameTimestamp && "iterator was invalidated during iteration");
Expand All @@ -277,7 +277,7 @@ class TransformState {
auto getPayloadValues(Value handleValue) const {
ArrayRef<Value> view = getPayloadValuesView(handleValue);

#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
// Memorize the current timestamp and make sure that it has not changed
// when incrementing or dereferencing the iterator returned by this
// function. The timestamp is incremented when the "values" mapping is
Expand Down
2 changes: 1 addition & 1 deletion mlir/include/mlir/IR/PDLPatternMatch.h.inc
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ template <typename PDLFnT, std::size_t... I>
void assertArgs(PatternRewriter &rewriter, ArrayRef<PDLValue> values,
std::index_sequence<I...>) {
// We only want to do verification in debug builds, same as with `assert`.
#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
using FnTraitsT = llvm::function_traits<PDLFnT>;
auto errorFn = [&](const Twine &msg) -> LogicalResult {
llvm::report_fatal_error(msg);
Expand Down
12 changes: 6 additions & 6 deletions mlir/lib/Dialect/Transform/Interfaces/TransformInterfaces.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ void transform::TransformState::forgetMapping(Value opHandle,
for (Operation *op : mappings.direct[opHandle])
dropMappingEntry(mappings.reverse, op, opHandle);
mappings.direct.erase(opHandle);
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
// Payload IR is removed from the mapping. This invalidates the respective
// iterators.
mappings.incrementTimestamp(opHandle);
Expand All @@ -342,7 +342,7 @@ void transform::TransformState::forgetMapping(Value opHandle,
for (Value resultHandle : resultHandles) {
Mappings &localMappings = getMapping(resultHandle);
dropMappingEntry(localMappings.values, resultHandle, opResult);
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
// Payload IR is removed from the mapping. This invalidates the respective
// iterators.
mappings.incrementTimestamp(resultHandle);
Expand All @@ -358,7 +358,7 @@ void transform::TransformState::forgetValueMapping(
for (Value payloadValue : mappings.reverseValues[valueHandle])
dropMappingEntry(mappings.reverseValues, payloadValue, valueHandle);
mappings.values.erase(valueHandle);
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
// Payload IR is removed from the mapping. This invalidates the respective
// iterators.
mappings.incrementTimestamp(valueHandle);
Expand All @@ -372,7 +372,7 @@ void transform::TransformState::forgetValueMapping(
dropMappingEntry(localMappings.direct, opHandle, payloadOp);
dropMappingEntry(localMappings.reverse, payloadOp, opHandle);

#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
// Payload IR is removed from the mapping. This invalidates the respective
// iterators.
localMappings.incrementTimestamp(opHandle);
Expand Down Expand Up @@ -452,7 +452,7 @@ transform::TransformState::replacePayloadValue(Value value, Value replacement) {
// between the handles and the IR objects
if (!replacement) {
dropMappingEntry(mappings.values, handle, value);
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
// Payload IR is removed from the mapping. This invalidates the respective
// iterators.
mappings.incrementTimestamp(handle);
Expand Down Expand Up @@ -804,7 +804,7 @@ checkRepeatedConsumptionInOperand(ArrayRef<T> payload,
void transform::TransformState::compactOpHandles() {
for (Value handle : opHandlesToCompact) {
Mappings &mappings = getMapping(handle, /*allowOutOfScope=*/true);
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
if (llvm::find(mappings.direct[handle], nullptr) !=
mappings.direct[handle].end())
// Payload IR is removed from the mapping. This invalidates the respective
Expand Down

0 comments on commit 45ad1ac

Please sign in to comment.