LLVM Discussion Forums

Use of "(void)regionIsIsolated;" statement in mlir::applyPatternsAndFoldGreedily(...)

What role does the statement:

(void)regionIsIsolated;

in the code:

bool mlir::applyPatternsAndFoldGreedily(
MutableArrayRef regions, const OwningRewritePatternList &patterns) {
if (regions.empty())
return true;

// The top-level operation must be known to be isolated from above to
// prevent performing canonicalizations on operations defined at or above
// the region containing ‘op’.
auto regionIsIsolated = [](Region &region) {
return region.getParentOp()->isKnownIsolatedFromAbove();
};
(void)regionIsIsolated;
assert(llvm::all_of(regions, regionIsIsolated) &&
“patterns can only be applied to operations IsolatedFromAbove”);

}

have? Is it there to prevent some optimization from taking place? Something else from C++?

Garrison

This variable is only used in assert, which is removed in Release builds, making the variable unused. Unused variables are a warning, promoted to a compiler error with -Werror that several MLIR users insist on enabling. Casting it to void is a common idiom to mark the variable as “used”.

Granted I don’t yet have an adequate understanding, but aren’t the contents of assert:

llvm::all_of(regions, regionIsIsolated)

still needed for release builds; the isolation check is still needed? I can see there is not a way to issue an error, especially for the case I’m looking at in the implementation of Canonicalizer::runOnOperation() where the return of applyPatternsAndFoldGreedily(…) is ignored.

Assertions are used for conditions that cannot happen: they check the internal consistency / invariants of the system. You can see them as a help for developers when modifying the system (ensuring you don’t break invariants) or when debugging.

https://llvm.org/docs/CodingStandards.html#assert-liberally

Thanks for the doc ref.

Unfortunately I don’t yet understand how OperationProperty::IsolatedFromAbove is an invariant for operations. I can see that both ModuleOp, and FunctionOp force OpTrait::IsIsolatedFromAbove, but I don’t yet see how IsolatedFromAbove (OpBase.td) is used/set for Toy’s TransposeOp for example.

Anyway I’ll look at this in more detail eventually. My real question was about the compiler warning use idiom employed.

Thanks! :slight_smile:

It’s not invariant for operations. We are generally referring to invariants of the code or system, like certain functions should never be called in certain dynamic conditions. So correct code would never call it, but incorrect may, and we need to warn about that. For example, one is not allowed to erase an operation if its results have uses. That too will trigger an assertion.

It is not used. It should not and actually cannot be used on it. If you read the documentation of this trait, it is related to regions contained in an Op, and there are none in TransposeOp. The tutorial is meant to merely let you start, it does not have an ambition to cover all possible APIs.

Yeah I realized after I wrote this that passes could not be applied to operations like TransposeOp since they don’t contain other operations (via regions) that could have passes applied to them (my words). Although this was not my original question, when you implied that “isolation” was invariant, I inevitably started to wonder about an example that would violate the assert or the release build noop because of the way a user constructed their operation. So I guess the user would have to create a new operation, that contained regions, and want to apply passes for that operation only. If the user forgot to mark the operation as isolated from above, maybe there would be an issue. Anyway this is abstract, and based on my lack of knowledge. Since in addition I don’t currently have the requisite knowledge to build such a test, I’m most likely belaboring an incorrect point. However your responses are definitely helpful to me. Thanks again for the references.

In this context, we usually would consider someone building a compiler with MLIR a “developer”: they are developing with assertions enabled so that it catches incorrect uses of APIs like this one. When they release their compiler to their users, the end-user of the compiler shouldn’t be able to construct arbitrary operations and directly run passes on them.

Ah ok this makes sense.