Skip to content

Commit af0fcf8

Browse files
authored
[mlir][tblgen] Don't echo absolute paths into rewrite pattern source (llvm#168984)
Currently, the declarative pattern rewrite generator will always print the [source]:[line](s) from which a pattern came. This is a useful debugging hint, but it causes problem when absolute paths are used as arguments to mlir-tblgen (which LLVM's build rules automatically do). Specifially, it causes the source to be tied to the build location, harning reproducability and our collective ability to get ccache hits from, say, separate worktrees. This commit resolves the issue by replacing absolute paths in thes "Generated from:" comments with their filenames. (The alternative would have been to implement an entire file-prefix-map the way the C compilers do, but since this is an isolated incident, I chose to resolve it locally.)
1 parent 0917a38 commit af0fcf8

File tree

3 files changed

+22
-7
lines changed

3 files changed

+22
-7
lines changed

mlir/include/mlir/TableGen/Pattern.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -643,8 +643,10 @@ class Pattern {
643643
using IdentifierLine = std::pair<StringRef, unsigned>;
644644

645645
// Returns the file location of the pattern (buffer identifier + line number
646-
// pair).
647-
std::vector<IdentifierLine> getLocation() const;
646+
// pair). If `forSourceOutput` is true, replace absolute paths in the buffer
647+
// identifier with just their filename so that we don't leak build paths into
648+
// the generated code.
649+
std::vector<IdentifierLine> getLocation(bool forSourceOutput = false) const;
648650

649651
// Recursively collects all bound symbols inside the DAG tree rooted
650652
// at `tree` and updates the given `infoMap`.

mlir/lib/TableGen/Pattern.cpp

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "llvm/ADT/Twine.h"
1919
#include "llvm/Support/Debug.h"
2020
#include "llvm/Support/FormatVariadic.h"
21+
#include "llvm/Support/Path.h"
2122
#include "llvm/TableGen/Error.h"
2223
#include "llvm/TableGen/Record.h"
2324

@@ -771,15 +772,27 @@ int Pattern::getBenefit() const {
771772
return initBenefit + dyn_cast<IntInit>(delta->getArg(0))->getValue();
772773
}
773774

774-
std::vector<Pattern::IdentifierLine> Pattern::getLocation() const {
775+
std::vector<Pattern::IdentifierLine>
776+
Pattern::getLocation(bool forSourceOutput) const {
775777
std::vector<std::pair<StringRef, unsigned>> result;
776778
result.reserve(def.getLoc().size());
777779
for (auto loc : def.getLoc()) {
778780
unsigned buf = llvm::SrcMgr.FindBufferContainingLoc(loc);
779781
assert(buf && "invalid source location");
780-
result.emplace_back(
781-
llvm::SrcMgr.getBufferInfo(buf).Buffer->getBufferIdentifier(),
782-
llvm::SrcMgr.getLineAndColumn(loc, buf).first);
782+
783+
StringRef bufferName =
784+
llvm::SrcMgr.getBufferInfo(buf).Buffer->getBufferIdentifier();
785+
// If we're emitting a generated file, we'd like to have some indication of
786+
// where our patterns came from. However, LLVM's build rules use absolute
787+
// paths as arguments to TableGen, and naively echoing such paths makes the
788+
// contents of the generated source file depend on the build location,
789+
// making MLIR builds substantially less reproducable. As a compromise, we
790+
// trim absolute paths back to only the filename component.
791+
if (forSourceOutput && llvm::sys::path::is_absolute(bufferName))
792+
bufferName = llvm::sys::path::filename(bufferName);
793+
794+
result.emplace_back(bufferName,
795+
llvm::SrcMgr.getLineAndColumn(loc, buf).first);
783796
}
784797
return result;
785798
}

mlir/tools/mlir-tblgen/RewriterGen.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1129,7 +1129,7 @@ void PatternEmitter::emit(StringRef rewriteName) {
11291129
LLVM_DEBUG(llvm::dbgs() << "done collecting ops used in result patterns\n");
11301130

11311131
// Emit RewritePattern for Pattern.
1132-
auto locs = pattern.getLocation();
1132+
auto locs = pattern.getLocation(/*forSourceOutput=*/true);
11331133
os << formatv("/* Generated from:\n {0:$[ instantiating\n ]}\n*/\n",
11341134
llvm::reverse(locs));
11351135
os << formatv(R"(struct {0} : public ::mlir::RewritePattern {

0 commit comments

Comments
 (0)