================
@@ -24,9 +27,140 @@ CIRGenModule::CIRGenModule(mlir::MLIRContext &context,
clang::ASTContext &astctx,
const clang::CodeGenOptions &cgo,
DiagnosticsEngine &diags)
- : astCtx(astctx), langOpts(astctx.getLangOpts()),
- theModule{mlir::ModuleOp::create(mlir::UnknownLoc())},
- target(astCtx.getTargetInfo()) {}
+ : builder(&context), astCtx(astctx), langOpts(astctx.getLangOpts()),
+ theModule{mlir::ModuleOp::create(mlir::UnknownLoc::get(&context))},
+ diags(diags), target(astCtx.getTargetInfo()) {}
+
+mlir::Location CIRGenModule::getLoc(SourceLocation cLoc) {
+ assert(cLoc.isValid() && "expected valid source location");
+ const SourceManager &sm = astCtx.getSourceManager();
+ PresumedLoc pLoc = sm.getPresumedLoc(cLoc);
+ StringRef filename = pLoc.getFilename();
+ return mlir::FileLineColLoc::get(builder.getStringAttr(filename),
+ pLoc.getLine(), pLoc.getColumn());
+}
----------------
AaronBallman wrote:
> SourceLocation is more of an implementation detail than mlir::Location is.
That depends on perspective, IMO. From our perspective, CIR is the way in which
we lower Clang's AST to an IR that eventually ends up in LLVM. The public
entrypoints into CIR from Clang's perspective should be using Clang's data
structures. If those need to be converted internally into MLIR-specific data
structures, that's fine, but that should not leak out into the public
interfaces that Clang can interact with.
However, I see now that I missed something important here -- this code is under
`clang/lib/CIR` and not `clang/include/clang/CIR`, so it *isn't* in the public
interfaces that Clang interacts with, it is (sufficiently) hidden as an
implementation detail. Sorry for missing that!
> We should solve the problem of getting good diagnostics during the MLIR
> passes when we actually run into the problem. It is only then that we will
> know how serious the problem is, how best to solve it, and how much effort to
> put into it. Solving that problem now is premature and will miss the mark in
> some way.
The point to these initial PRs is to ensure the community is comfortable with
the design and doesn't see any glaring design concerns, so saying "we'll solve
it when we get to it" on a situation we know we will hit sort of defeats the
purpose. We already know the problem exists because it's one we fight with
today and we already know a better way to solve it is to use source location
information with at least as much fidelity as Clang is able to handle. Given
that one of the benefits to CIR is for better codegen analysis that can lead to
improved diagnostics, having this as an early design concern is pretty
reasonable IMO. That said, we're not insisting on a change as part of this PR
-- just that the CIR folks understand that this is a design concern with the
facilities and at some point it may go from "concern" to "required to improve
before we can proceed."
https://github.com/llvm/llvm-project/pull/113483
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
