Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions mlir/include/mlir/IR/Builders.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class AffineExpr;
class IRMapping;
class UnknownLoc;
class FileLineColLoc;
class FileLineColRange;
class Type;
class PrimitiveType;
class IntegerType;
Expand Down
24 changes: 20 additions & 4 deletions mlir/include/mlir/IR/BuiltinDialectBytecode.td
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,26 @@ def CallSiteLoc : DialectAttribute<(attr
LocationAttr:$caller
)>;

let cType = "FileLineColRange" in {
def FileLineColRange : DialectAttribute<(attr
StringAttr:$filename,
WithBuilder<"$_args",
WithType<"SmallVector<uint64_t>",
WithParser <"succeeded(readFileLineColRangeLocs($_reader, $_var))",
WithPrinter<"writeFileLineColRangeLocs($_writer, $_name)">>>>:$rawLocData
)> {
let cBuilder = "getFileLineColRange(context, filename, rawLocData)";
let printerPredicate = "!::llvm::isa<FileLineColLoc>($_val)";
}

def FileLineColLoc : DialectAttribute<(attr
StringAttr:$filename,
VarInt:$line,
VarInt:$column
)>;
PresentOptionalVarInt:$start_line,
PresentOptionalVarInt:$start_column
)> {
let printerPredicate = "::llvm::isa<FileLineColLoc>($_val)";
}
}

let cType = "FusedLoc",
cBuilder = "cast<FusedLoc>(get<FusedLoc>(context, $_args))" in {
Expand Down Expand Up @@ -321,7 +336,8 @@ def BuiltinDialectAttributes : DialectAttributes<"Builtin"> {
DenseIntOrFPElementsAttr,
DenseStringElementsAttr,
SparseElementsAttr,
DistinctAttr
DistinctAttr,
FileLineColRange,
];
}

Expand Down
83 changes: 67 additions & 16 deletions mlir/include/mlir/IR/BuiltinLocationAttributes.td
Original file line number Diff line number Diff line change
Expand Up @@ -60,46 +60,97 @@ def CallSiteLoc : Builtin_LocationAttr<"CallSiteLoc"> {
}

//===----------------------------------------------------------------------===//
// FileLineColLoc
// FileLineColRange
//===----------------------------------------------------------------------===//

def FileLineColLoc : Builtin_LocationAttr<"FileLineColLoc"> {
let summary = "A file:line:column source location";
def FileLineColRange : Builtin_LocationAttr<"FileLineColRange"> {
let summary = "A file:line:column source location range";
let description = [{
Syntax:

```
filelinecol-location ::= string-literal `:` integer-literal `:`
integer-literal
(`to` (integer-literal ?) `:` integer-literal ?)
```

An instance of this location represents a tuple of file, line number, and
column number. This is similar to the type of location that you get from
most source languages.
An instance of this location represents a tuple of file, start and end line
number, and start and end column number. It allows for the following
configurations:

* A single file line location: `file:line`;
* A single file line col location: `file:line:column`;
* A single line range: `file:line:column to :column`;
* A single file range: `file:line:column to line:column`;

Example:

```mlir
loc("mysource.cc":10:8)
loc("mysource.cc":10:8 to 12:18)
```
}];
let parameters = (ins "StringAttr":$filename, "unsigned":$line,
"unsigned":$column);

// Note: this only shows the parameters for which accessors are generated. The
// locations are only set in storage.
let parameters = (ins "StringAttr":$filename);
let builders = [

AttrBuilderWithInferredContext<(ins "StringAttr":$filename), [{
return $_get(filename.getContext(), filename, ArrayRef<unsigned>{});
}]>,
AttrBuilderWithInferredContext<(ins "StringAttr":$filename,
"unsigned":$line), [{
return $_get(filename.getContext(), filename,
ArrayRef<unsigned>{line});
}]>,
AttrBuilderWithInferredContext<(ins "StringAttr":$filename,
"unsigned":$line,
"unsigned":$column), [{
return $_get(filename.getContext(), filename, line, column);
return $_get(filename.getContext(), filename,
ArrayRef<unsigned>{line, column});
}]>,
AttrBuilder<(ins "StringRef":$filename, "unsigned":$line,
"unsigned":$column), [{
AttrBuilder<(ins "::llvm::StringRef":$filename,
"unsigned":$start_line,
"unsigned":$start_column), [{
return $_get($_ctxt,
StringAttr::get($_ctxt, filename.empty() ? "-" : filename),
line, column);
}]>
StringAttr::get($_ctxt, filename.empty() ? "-" : filename),
ArrayRef<unsigned>{start_line, start_column});
}]>,
AttrBuilderWithInferredContext<(ins "::mlir::StringAttr":$filename,
"unsigned":$line,
"unsigned":$start_column,
"unsigned":$end_column), [{
return $_get(filename.getContext(), filename,
ArrayRef<unsigned>{line, start_column, end_column});
}]>,
AttrBuilderWithInferredContext<(ins "::mlir::StringAttr":$filename,
"unsigned":$start_line,
"unsigned":$start_column,
"unsigned":$end_line,
"unsigned":$end_column), [{
return $_get(filename.getContext(), filename,
ArrayRef<unsigned>{start_line, start_column, end_column, end_line});
}]>,
AttrBuilder<(ins "::llvm::StringRef":$filename,
"unsigned":$start_line,
"unsigned":$start_column,
"unsigned":$end_line,
"unsigned":$end_column), [{
return $_get($_ctxt,
StringAttr::get($_ctxt, filename.empty() ? "-" : filename),
ArrayRef<unsigned>{start_line, start_column, end_column, end_line});
}]>,
];

let extraClassDeclaration = [{
std::optional<unsigned> getStartLine() const;
std::optional<unsigned> getStartColumn() const;
std::optional<unsigned> getEndColumn() const;
std::optional<unsigned> getEndLine() const;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the use of optionality here for expressiveness, but I actually wonder if this is just going to make the class clunky to interact with. Could we rework these accessors to always return a valid integer? I think we could do that while maintaining the internal optimization for the different situations, e.g.:

*   A single file location: `file`;
    * Everything returns 0.
*   A single file line location: `file:line`;
    * Columns return 0, end line returns `line`.
*   A single file line col location: `file:line:column`;
    * End line returns `line`, end column returns `column`.
*   A single line range: `file:line:column to :column`;
    * End line returns `line`.
*   A single file range: `file:line:column to line:column`;
    * N/A.

I'd like to avoid the code that is in the parser (i.e. all of the "if present") in user code as much as possible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SG, I considered this but thought it may be simpler to be explicit. But don't have good rationale for when it may be needed, so switching to simpler. I don't want to expose the in memory encoding, so doing some extra work in the bytecode writer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm, on second thought. I'm not sure if not using optional doesn't result in requiring more folks to be aware of the convention or encoding. Now if I have question "is this actually a range", a consumer would have to know the above. Which might not be an issue if all consumers treat empty ranges as locations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to map this to all of the other source ranges I know of (SM, clang, lsp, etc.), none of which use optional. In which situation are you thinking this might cause an issue? I haven't yet really encountered anything problematic so far when using the other source ranges I know of.

Copy link
Member Author

@jpienaar jpienaar Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its just difference between single location vs range that needs to be differentiated really (and I could just use an isa on FileLineColLoc for that I realized this morning). For the hard coding between bytecode format and what's in memory, we could consider that fine as internal.

Actually that's incidental (bytecode): one could change one and not the other due to using the switch these are decoupled. Bytecode internally just has to be consistent.

}];
let skipDefaultBuilders = 1;
let attrName = "builtin.file_line_loc";
let genStorageClass = 0;
let attrName = "builtin.file_line_range";
}

//===----------------------------------------------------------------------===//
Expand Down
6 changes: 6 additions & 0 deletions mlir/include/mlir/IR/BytecodeBase.td
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@ def VarInt :
WithBuilder<"$_args",
WithPrinter<"$_writer.writeVarInt($_getter)",
WithType <"uint64_t">>>>;
// This is for optional ints which are guaranteed to be present.
def PresentOptionalVarInt :
WithParser <"succeeded($_reader.readVarInt($_var))",
WithBuilder<"$_args",
WithPrinter<"$_writer.writeVarInt(*$_getter)",
WithType <"uint64_t">>>>;
def SignedVarInt :
WithParser <"succeeded($_reader.readSignedVarInt($_var))",
WithBuilder<"$_args",
Expand Down
31 changes: 31 additions & 0 deletions mlir/include/mlir/IR/Location.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ inline ::llvm::hash_code hash_value(Location arg) {
// Tablegen Attribute Declarations
//===----------------------------------------------------------------------===//

// Forward declaration for class created later.
namespace mlir::detail {
struct FileLineColRangeAttrStorage;
} // namespace mlir::detail

#define GET_ATTRDEF_CLASSES
#include "mlir/IR/BuiltinLocationAttributes.h.inc"

Expand Down Expand Up @@ -164,6 +169,32 @@ class FusedLocWith : public FusedLoc {
}
};

//===----------------------------------------------------------------------===//
// FileLineColLoc
//===----------------------------------------------------------------------===//

// An instance of this location represents a tuple of file, line number, and
// column number. This is similar to the type of location that you get from
// most source languages.
//
// FileLineColLoc is a FileLineColRange with exactly one line and column.
class FileLineColLoc : public FileLineColRange {
public:
using FileLineColRange::FileLineColRange;

static FileLineColLoc get(StringAttr filename, unsigned line,
unsigned column);
static FileLineColLoc get(MLIRContext *context, StringRef fileName,
unsigned line, unsigned column);

StringAttr getFilename() const;
unsigned getLine() const;
unsigned getColumn() const;

/// Methods for support type inquiry through isa, cast, and dyn_cast.
static bool classof(Attribute attr);
};

//===----------------------------------------------------------------------===//
// OpaqueLoc
//===----------------------------------------------------------------------===//
Expand Down
74 changes: 60 additions & 14 deletions mlir/lib/AsmParser/LocationParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "mlir/IR/BuiltinAttributes.h"
#include "mlir/IR/Location.h"
#include "mlir/Support/LLVM.h"
#include <optional>

using namespace mlir;
using namespace mlir::detail;
Expand Down Expand Up @@ -97,37 +98,82 @@ ParseResult Parser::parseFusedLocation(LocationAttr &loc) {
return success();
}

ParseResult Parser::parseNameOrFileLineColLocation(LocationAttr &loc) {
ParseResult Parser::parseNameOrFileLineColRange(LocationAttr &loc) {
auto *ctx = getContext();
auto str = getToken().getStringValue();
consumeToken(Token::string);

std::optional<unsigned> startLine, startColumn, endLine, endColumn;

// If the next token is ':' this is a filelinecol location.
if (consumeIf(Token::colon)) {
// Parse the line number.
if (getToken().isNot(Token::integer))
return emitWrongTokenError(
"expected integer line number in FileLineColLoc");
auto line = getToken().getUnsignedIntegerValue();
if (!line)
"expected integer line number in FileLineColRange");
startLine = getToken().getUnsignedIntegerValue();
if (!startLine)
return emitWrongTokenError(
"expected integer line number in FileLineColLoc");
"expected integer line number in FileLineColRange");
consumeToken(Token::integer);

// Parse the ':'.
if (parseToken(Token::colon, "expected ':' in FileLineColLoc"))
return failure();
if (getToken().isNot(Token::colon)) {
loc = FileLineColRange::get(StringAttr::get(ctx, str), *startLine);
return success();
}
consumeToken(Token::colon);

// Parse the column number.
if (getToken().isNot(Token::integer))
if (getToken().isNot(Token::integer)) {
return emitWrongTokenError(
"expected integer column number in FileLineColRange");
}
startColumn = getToken().getUnsignedIntegerValue();
if (!startColumn.has_value())
return emitError("expected integer column number in FileLineColRange");
consumeToken(Token::integer);

if (!isCurrentTokenAKeyword() || getTokenSpelling() != "to") {
loc = FileLineColLoc::get(ctx, str, *startLine, *startColumn);
return success();
}
consumeToken();

// Parse the line number.
if (getToken().is(Token::integer)) {
endLine = getToken().getUnsignedIntegerValue();
if (!endLine) {
return emitWrongTokenError(
"expected integer line number in FileLineColRange");
}
consumeToken(Token::integer);
}

// Parse the ':'.
if (getToken().isNot(Token::colon)) {
return emitWrongTokenError(
"expected either integer or `:` post `to` in FileLineColRange");
}
consumeToken(Token::colon);

// Parse the column number.
if (getToken().isNot(Token::integer)) {
return emitWrongTokenError(
"expected integer column number in FileLineColLoc");
auto column = getToken().getUnsignedIntegerValue();
if (!column.has_value())
return emitError("expected integer column number in FileLineColLoc");
"expected integer column number in FileLineColRange");
}
endColumn = getToken().getUnsignedIntegerValue();
if (!endColumn.has_value())
return emitError("expected integer column number in FileLineColRange");
consumeToken(Token::integer);

loc = FileLineColLoc::get(ctx, str, *line, *column);
if (endLine.has_value()) {
loc = FileLineColRange::get(StringAttr::get(ctx, str), *startLine,
*startColumn, *endLine, *endColumn);
} else {
loc = FileLineColRange::get(StringAttr::get(ctx, str), *startLine,
*startColumn, *endColumn);
}
return success();
}

Expand Down Expand Up @@ -166,7 +212,7 @@ ParseResult Parser::parseLocationInstance(LocationAttr &loc) {

// Handle either name or filelinecol locations.
if (getToken().is(Token::string))
return parseNameOrFileLineColLocation(loc);
return parseNameOrFileLineColRange(loc);

// Bare tokens required for other cases.
if (!getToken().is(Token::bare_identifier))
Expand Down
2 changes: 1 addition & 1 deletion mlir/lib/AsmParser/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ class Parser {
ParseResult parseFusedLocation(LocationAttr &loc);

/// Parse a name or FileLineCol location instance.
ParseResult parseNameOrFileLineColLocation(LocationAttr &loc);
ParseResult parseNameOrFileLineColRange(LocationAttr &loc);

//===--------------------------------------------------------------------===//
// Affine Parsing
Expand Down
13 changes: 11 additions & 2 deletions mlir/lib/IR/AsmPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2009,12 +2009,21 @@ void AsmPrinter::Impl::printLocationInternal(LocationAttr loc, bool pretty,
else
os << "unknown";
})
.Case<FileLineColLoc>([&](FileLineColLoc loc) {
.Case<FileLineColRange>([&](FileLineColRange loc) {
if (pretty)
os << loc.getFilename().getValue();
else
printEscapedString(loc.getFilename());
os << ':' << loc.getLine() << ':' << loc.getColumn();
os << ':' << *loc.getStartLine();
if (loc.getStartColumn()) {
os << ':' << *loc.getStartColumn();
if (loc.getEndColumn().has_value() || loc.getEndLine().has_value())
os << " to ";
if (loc.getEndLine().has_value())
os << *loc.getEndLine();
if (loc.getEndColumn().has_value())
os << ':' << *loc.getEndColumn();
}
})
.Case<NameLoc>([&](NameLoc loc) {
printEscapedString(loc.getName());
Expand Down
Loading
Loading