Skip to content

Commit 55cf53f

Browse files
committed
[mlir][Parser] Make parse{Attribute,Type} null-terminate input
`parseAttribute` and `parseType` require null-terminated strings as input, but this isn't great considering the argument type is `StringRef`. This changes them to copy to a null-terminated buffer by default, with a `isKnownNullTerminated` flag added to disable the copying. closes llvm#58964 Reviewed By: rriddle, kuhar, lattner Differential Revision: https://reviews.llvm.org/D145182
1 parent ae12e57 commit 55cf53f

File tree

6 files changed

+34
-18
lines changed

6 files changed

+34
-18
lines changed

mlir/include/mlir/AsmParser/AsmParser.h

+7-2
Original file line numberDiff line numberDiff line change
@@ -49,16 +49,21 @@ parseAsmSourceFile(const llvm::SourceMgr &sourceMgr, Block *block,
4949
/// If `numRead` is provided, it is set to the number of consumed characters on
5050
/// succesful parse. Otherwise, parsing fails if the entire string is not
5151
/// consumed.
52+
/// Some internal copying can be skipped if the source string is known to be
53+
/// null terminated.
5254
Attribute parseAttribute(llvm::StringRef attrStr, MLIRContext *context,
53-
Type type = {}, size_t *numRead = nullptr);
55+
Type type = {}, size_t *numRead = nullptr,
56+
bool isKnownNullTerminated = false);
5457

5558
/// This parses a single MLIR type to an MLIR context if it was valid. If not,
5659
/// an error diagnostic is emitted to the context.
5760
/// If `numRead` is provided, it is set to the number of consumed characters on
5861
/// succesful parse. Otherwise, parsing fails if the entire string is not
5962
/// consumed.
63+
/// Some internal copying can be skipped if the source string is known to be
64+
/// null terminated.
6065
Type parseType(llvm::StringRef typeStr, MLIRContext *context,
61-
size_t *numRead = nullptr);
66+
size_t *numRead = nullptr, bool isKnownNullTerminated = false);
6267

6368
/// This parses a single IntegerSet/AffineMap to an MLIR context if it was
6469
/// valid. If not, an error message is emitted through a new

mlir/lib/AsmParser/DialectSymbolParser.cpp

+14-9
Original file line numberDiff line numberDiff line change
@@ -306,15 +306,18 @@ Type Parser::parseExtendedType() {
306306
//===----------------------------------------------------------------------===//
307307

308308
/// Parses a symbol, of type 'T', and returns it if parsing was successful. If
309-
/// parsing failed, nullptr is returned. The number of bytes read from the input
310-
/// string is returned in 'numRead'.
309+
/// parsing failed, nullptr is returned.
311310
template <typename T, typename ParserFn>
312311
static T parseSymbol(StringRef inputStr, MLIRContext *context,
313-
size_t *numReadOut, ParserFn &&parserFn) {
312+
size_t *numReadOut, bool isKnownNullTerminated,
313+
ParserFn &&parserFn) {
314314
// Set the buffer name to the string being parsed, so that it appears in error
315315
// diagnostics.
316-
auto memBuffer = MemoryBuffer::getMemBuffer(inputStr, /*BufferName=*/inputStr,
317-
/*RequiresNullTerminator=*/true);
316+
auto memBuffer =
317+
isKnownNullTerminated
318+
? MemoryBuffer::getMemBuffer(inputStr,
319+
/*BufferName=*/inputStr)
320+
: MemoryBuffer::getMemBufferCopy(inputStr, /*BufferName=*/inputStr);
318321
SourceMgr sourceMgr;
319322
sourceMgr.AddNewSourceBuffer(std::move(memBuffer), SMLoc());
320323
SymbolState aliasState;
@@ -343,12 +346,14 @@ static T parseSymbol(StringRef inputStr, MLIRContext *context,
343346
}
344347

345348
Attribute mlir::parseAttribute(StringRef attrStr, MLIRContext *context,
346-
Type type, size_t *numRead) {
349+
Type type, size_t *numRead,
350+
bool isKnownNullTerminated) {
347351
return parseSymbol<Attribute>(
348-
attrStr, context, numRead,
352+
attrStr, context, numRead, isKnownNullTerminated,
349353
[type](Parser &parser) { return parser.parseAttribute(type); });
350354
}
351-
Type mlir::parseType(StringRef typeStr, MLIRContext *context, size_t *numRead) {
352-
return parseSymbol<Type>(typeStr, context, numRead,
355+
Type mlir::parseType(StringRef typeStr, MLIRContext *context, size_t *numRead,
356+
bool isKnownNullTerminated) {
357+
return parseSymbol<Type>(typeStr, context, numRead, isKnownNullTerminated,
353358
[](Parser &parser) { return parser.parseType(); });
354359
}

mlir/lib/Bytecode/Reader/BytecodeReader.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -1031,9 +1031,11 @@ LogicalResult AttrTypeReader::parseAsmEntry(T &result, EncodingReader &reader,
10311031
size_t numRead = 0;
10321032
MLIRContext *context = fileLoc->getContext();
10331033
if constexpr (std::is_same_v<T, Type>)
1034-
result = ::parseType(asmStr, context, &numRead);
1034+
result =
1035+
::parseType(asmStr, context, &numRead, /*isKnownNullTerminated=*/true);
10351036
else
1036-
result = ::parseAttribute(asmStr, context, Type(), &numRead);
1037+
result = ::parseAttribute(asmStr, context, Type(), &numRead,
1038+
/*isKnownNullTerminated=*/true);
10371039
if (!result)
10381040
return failure();
10391041

mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -1693,7 +1693,8 @@ transform::PadOp::applyToOne(LinalgOp target,
16931693
// Try to parse string attributes to obtain an attribute of element type.
16941694
if (auto stringAttr = attr.dyn_cast<StringAttr>()) {
16951695
auto parsedAttr = dyn_cast_if_present<TypedAttr>(
1696-
parseAttribute(stringAttr, getContext(), elementType));
1696+
parseAttribute(stringAttr, getContext(), elementType,
1697+
/*numRead=*/nullptr, /*isKnownNullTerminated=*/true));
16971698
if (!parsedAttr || parsedAttr.getType() != elementType) {
16981699
auto diag = this->emitOpError("expects a padding that parses to ")
16991700
<< elementType << ", got " << std::get<0>(it);

mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-yaml-gen.cpp

+2-4
Original file line numberDiff line numberDiff line change
@@ -317,10 +317,8 @@ struct ScalarTraits<SerializedAffineMap> {
317317
SerializedAffineMap &value) {
318318
assert(rawYamlContext);
319319
auto *yamlContext = static_cast<LinalgYAMLContext *>(rawYamlContext);
320-
std::string nullTerminatedScalar(scalar);
321-
if (auto attr =
322-
mlir::parseAttribute(nullTerminatedScalar, yamlContext->mlirContext)
323-
.dyn_cast_or_null<AffineMapAttr>())
320+
if (auto attr = mlir::parseAttribute(scalar, yamlContext->mlirContext)
321+
.dyn_cast_or_null<AffineMapAttr>())
324322
value.affineMapAttr = attr;
325323
else if (!value.affineMapAttr || !value.affineMapAttr.isa<AffineMapAttr>())
326324
return "could not parse as an affine map attribute";

mlir/unittests/Parser/ParserTest.cpp

+5
Original file line numberDiff line numberDiff line change
@@ -95,5 +95,10 @@ TEST(MLIRParser, ParseAttr) {
9595
b.getI64IntegerAttr(10));
9696
EXPECT_EQ(numRead, size_t(4)); // includes trailing whitespace
9797
}
98+
{ // Parse without null-terminator
99+
StringRef attrAsm("999", 1);
100+
Attribute attr = parseAttribute(attrAsm, &context);
101+
EXPECT_EQ(attr, b.getI64IntegerAttr(9));
102+
}
98103
}
99104
} // namespace

0 commit comments

Comments
 (0)