- Extend the Symbol interface with isDeclaration to identify operations that declare a symbol as opposed to define it.
- Extend verification to disallow public declarations as per the discussion in https://llvm.discourse.group/t/rfc-symbol-definition-declaration-x-visibility-checks/2140
- Adopt the new interface for FuncOp and fix test and code to not have/create public function declarations.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/docs/SymbolsAndSymbolTables.md | ||
---|---|---|
56 | It is a bit hard to find the right language here: from a SymbolTable point of view a new symbol is defined, but it is a placeholder for an entity defined outside of the current scope. |
mlir/integration_test/Sparse/CPU/matrix-market-example.mlir | ||
---|---|---|
14 | Actually I wonder if we shouldn't just drop it for declaration and consider it the default? |
mlir/docs/SymbolsAndSymbolTables.md | ||
---|---|---|
56 | Agreed, I am open to suggestions here. Though, the spec language does not say anywhere that symbols are "defined" in a symbol table. For example, the next section says:
So there is no notion of a SymbolTable "defining" a symbol at least reading the spec language here. That being said, if this can be clarified better, we should. | |
mlir/integration_test/Sparse/CPU/matrix-market-example.mlir | ||
14 | I know I had brought it up earlier on discourse. Two things:
In any case, I think the parser will always accept explicit visibility when present, so for now I am proposing sticking with this. |
mlir/include/mlir/IR/Function.h | ||
---|---|---|
92 | Note, this will eventually change when we eliminate all existing uses of isExternal(). So please consider this as staging for now. |
It is a bit hard to find the right language here: from a SymbolTable point of view a new symbol is defined, but it is a placeholder for an entity defined outside of the current scope.