This assertion does not achieve what it meant to do originally, as it
would fire only when applied to an unregistered operation, which is a
fairly rare circumstance (it needs a dialect or context allowing
unregistered operation in the input in the first place).
Instead we relax it to only fire when it should have matched but didn't
because of the misconfiguration.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
The relationship between the op classes and registration feels under documented and easily error prone to new users. The fact that isa<AffineForOp>(op) will silently fail on an affine.for operation because the affine dialect isn't registered will likely feel wrong to many as an initial reaction. I'm not saying we should allow or encourage the use of the *Op classes when something is unregistered(we shouldn't), but it feels like we could make debugging/understanding a misconfiguration much easier; which if I recall is the original intention of the assert. E.g. as a bare bone hint, could we add a debug log that detects such situations and issues a message? e.g. something like:
LLVM_DEBUG({ if (op->getName().getStringRef() == ConcreteType::getOperationName()) dbgs() << "classof on '" << ConcreteType::getOperationName() << "' failed due to the operation not being registered\n"; })
The fact that isa<AffineForOp>(op) will silently fail on an affine.for operation because the affine dialect isn't registered will likely feel wrong to many as an initial reaction
My reasoning is that by default we don't allow to create an unregistered AffineForOp in the IR in the first place: the situation you're describing can only happen if the user use --allow-unregistered-dialects I believe.
But your suggestion seems fine as well for such cases!