[clang] [CIR] Call code gen; create empty cir.func op (PR #113483)

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 5 05:41:01 PST 2024


================
@@ -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


More information about the cfe-commits mailing list