This is an archive of the discontinued LLVM Phabricator instance.

[mlir][ods] Fix StringRef initialization in builders
ClosedPublic

Authored by antiagainst on Jan 18 2020, 7:37 AM.

Details

Summary

For the generated builder taking in unwrapped attribute values,
if the argument is a string, we should avoid wrapping it in quotes;
otherwise we are always setting the string attribute to contain
the string argument's name. The quotes come from StrinAttr's
constBuilderCall, which is reasonable for string literals, but
not function arguments containing strings.

Diff Detail

Event Timeline

antiagainst created this revision.Jan 18 2020, 7:37 AM

Revise comment

Unit tests: pass. 61989 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Unit tests: pass. 61989 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

rriddle requested changes to this revision.Jan 18 2020, 12:23 PM
rriddle added inline comments.
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
98

Why not just add a replaceAllSubStrs using std::string instead? Generally only takes ~5 lines of code.

This revision now requires changes to proceed.Jan 18 2020, 12:23 PM

Address comment

antiagainst marked 2 inline comments as done.Jan 21 2020, 6:47 AM
antiagainst added inline comments.
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
98

Sure, that works too. Changed.

Unit tests: pass. 61989 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

rriddle accepted this revision.Jan 21 2020, 10:49 AM
rriddle added inline comments.
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
990

Remove trivial brace.

This revision is now accepted and ready to land.Jan 21 2020, 10:49 AM
This revision was automatically updated to reflect the committed changes.
antiagainst marked an inline comment as done.