Ok, synced with Aman offline, so to summarize discussion/sanity check my understanding,
and what should be done.
The initial DILocationAttr proposal stores a FileLineColLoc (file + line +
col) alongside a DILocalScopeAttr. The scope contains a DIFileAttr which
also carries the filename. So the file identity is stored twice with nothing
enforcing they match.
This duplication is not new — the existing FusedLoc representation has the
same issue. FusedLocWith<DISubprogramAttr>({FileLineColLoc(...)}, subprogram)
carries the filename in the FileLineColLoc child and again in the
subprogram’s DIFileAttr. Every consumer extracts the filename from
FileLineColLoc and ignores what’s in the scope. Introducing DILocationAttr
gives us a chance to clean this up.
The solution is to have DILocationAttr store only line + column + scope
(no FileLineColLoc) and derive the filename from the scope’s DIFileAttr.
This mirrors LLVM IR’s DILocation, which stores line, column, and scope, and
gets the filename via getScope()->getFilename().
The problem is that MLIR infrastructure is
hardcoded to FileLineColLoc — it uses findInstanceOf<FileLineColLoc>() and
dyn_cast<FileLineColLoc>() to extract file/line/column. A DILocationAttr
that doesn’t contain a FileLineColLoc would be invisible to all of this.
The fix is a FileLineColRangeInterface that both FileLineColRange and
DILocationAttr implement. Consumers switch from the concrete type to the
interface, and any location that provides file/line/column works automatically.
On LLVM level DILocation doesn’t have a concept of ranges, but Aman mentioned
Clang does. So it doesn’t hurt to keep it generic/future proof.
Back of the napkin implementation.
Define an AttrInterface in BuiltinAttributeInterfaces.td that any location
attribute can implement to say “I carry file/line/column info”:
def FileLineColRangeInterface : AttrInterface<"FileLineColRangeInterface"> {
let description = [{
Interface for location attributes that carry file, line, and column info.
}];
let cppNamespace = "::mlir";
let methods = [
InterfaceMethod<"Get the filename", "::mlir::StringAttr", "getFilename">,
InterfaceMethod<"Get the start line", "unsigned", "getStartLine">,
InterfaceMethod<"Get the start column", "unsigned", "getStartColumn">,
InterfaceMethod<"Get the end line", "unsigned", "getEndLine">,
InterfaceMethod<"Get the end column", "unsigned", "getEndColumn">,
];
let extraClassDeclaration = [{
unsigned getLine() { return getStartLine(); }
unsigned getColumn() { return getStartColumn(); }
}];
}
Have FileLineColRange implement it — trivial since it already has all these
methods. FileLineColLoc inherits it for free.
Then update the handful of places in MLIR that currently hardcode
FileLineColLoc to use the interface type instead. Behavior stays identical — the only
implementor at this stage is FileLineColRange. The point is that the system is
now open, so when DILocationAttr implements the interface in a follow-up PR,
all these consumers pick it up automatically with zero additional changes.
How DILocationAttr will look after this
With the interface in place, DILocationAttr drops FileLineColLoc and stores
only what LLVM IR’s DILocation stores — line, column, and scope:
def LLVM_DILocationAttr : LocationAttrDef<LLVM_Dialect, "DILocation",
[DeclareAttrInterfaceMethods<FileLineColRangeInterface>]> {
let mnemonic = "di_location";
let parameters = (ins
"unsigned":$line,
"unsigned":$column,
"DILocalScopeAttr":$scope
);
let assemblyFormat = "`<` $line `:` $column `in` $scope `>`";
}
The interface methods are implemented by:
getLine() / getColumn() — return the stored fields directly
getFilename() — delegate to the scope: getScope().getFile().getName(),
same as LLVM’s DILocation::getFilename() calls getScope()->getFilename()
getStartLine() / getEndLine() — both return getLine() (it’s a point)
getStartColumn() / getEndColumn() — both return getColumn()
Result no file duplication. The filename lives only in the scope’s DIFileAttr.
Diagnostics, breakpoints, and other infra see DILocationAttr through the
interface and extract file/line/column without knowing or caring about the
concrete type.