[Mlir-commits] [mlir] [mlir] Add loaded URI attribute type (PR #67276)

[Jacques Pienaar]

jpienaar wrote:

> It's not clear to me that this should be a builtin here: seems a bit quite specific?

Not sure I agree, I see this as no more specific than DenseElementsAttr.

> 
> Also the current implementation only support one scheme, the "URL" is implicitly a file path apparently, and it's eagerly loaded. Adding more scheme seems necessarily intrusive here (you have to modify the builtin dialect): there does not seem to be any dialect-customization available.

URI for now is implicitly a file path yes. This would later become the fallback behavior (well as there has to be some). So you'd switch on scheme of the URI (here currently only file is supported so no switching or checking) and as fallback you make some attempt (and there loading a file is probably the simplest).

The plan here is to have a static registry method on the type (that's the TODO there). This would allow external groups to register a handler without changing the builtin dialect (e.g., `LoadedURIResourceAttr::register(foo)`). Now registries mean that it is up to the tool/flow to have the correct ones registered, which is not great and I think we could add some builtin support, but also not bad as we don't really want to handle all file types here vs giving downstream users a choice. The storage here is same as builtin resource, with the URI handlers not linked to a dialect but instead would handled instead by one of the functors - the end result should be a loaded resource in the correct layout.

So eagerly loaded I call out in the PR description as something that could be added. I am in between on it honestly. I figured one could do it lazily first time accessed (make it mutable attribute), that has the downside that your IR may pass verification multiple times before it actually checks if URI can be loaded and fails. I figured that we can adjust based on actual data.

> 
> I am wondering if the value here isn't in the mechanic more than the attribute itself: that is we could just provide the facilities for dialect to have this behavior easily, without providing an actual attribute?

Could you expand? Similar to the resource blob kind, I think >90% of the cases may be handled by this without dialect specialization. Common cases will probably be a splatted file of constants or MLIR Bytecode, the for the more application focussed formats (e.g., safetensor and the like) one may get lucky with the raw format even (many of them are made to be mmapable, we may need to add a layout attribute there ... but I haven't tried that). And so the dialect doesn't have much impact, its just an ElementsAttr where we can avoid opening the same file redundantly.

https://github.com/llvm/llvm-project/pull/67276