[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,88 @@
+//===--- CIRGenAction.cpp - LLVM Code generation Frontend Action -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/CIRFrontendAction/CIRGenAction.h"
+#include "clang/CIR/CIRGenerator.h"
+#include "clang/Frontend/CompilerInstance.h"
+
+#include "mlir/IR/MLIRContext.h"
+#include "mlir/IR/OwningOpRef.h"
+
+using namespace cir;
+using namespace clang;
+
+namespace cir {
+
+class CIRGenConsumer : public clang::ASTConsumer {
+
+ virtual void anchor();
+
+ [[maybe_unused]] CIRGenAction::OutputType action;
+
+ [[maybe_unused]] DiagnosticsEngine
+ [[maybe_unused]] const HeaderSearchOptions
+ [[maybe_unused]] const CodeGenOptions
+ [[maybe_unused]] const TargetOptions
+ [[maybe_unused]] const LangOptions
+ [[maybe_unused]] const FrontendOptions
+
+ std::unique_ptr outputStream;
+
+ [[maybe_unused]] ASTContext *astContext{nullptr};
+ IntrusiveRefCntPtr FS;
+ std::unique_ptr gen;
+
+public:
+ CIRGenConsumer(CIRGenAction::OutputType action,
+ DiagnosticsEngine ,
+ IntrusiveRefCntPtr VFS,
+ const HeaderSearchOptions ,
+ const CodeGenOptions ,
+ const TargetOptions ,
+ const LangOptions ,
+ const FrontendOptions ,
+ std::unique_ptr os)
+ : action(action), diagnosticsEngine(diagnosticsEngine),
+headerSearchOptions(headerSearchOptions),
+codeGenOptions(codeGenOptions), targetOptions(targetOptions),
+langOptions(langOptions), feOptions(feOptions),
+outputStream(std::move(os)), FS(VFS),
+gen(std::make_unique(diagnosticsEngine, std::move(VFS),
+ codeGenOptions)) {}
+
+ bool HandleTopLevelDecl(DeclGroupRef D) override {
+gen->HandleTopLevelDecl(D);
+return true;
+ }
+};
+} // namespace cir
+
+void CIRGenConsumer::anchor() {}
+
+CIRGenAction::CIRGenAction(OutputType act, mlir::MLIRContext *mlirContext)
+: mlirContext(mlirContext ? mlirContext : new mlir::MLIRContext),
+ action(act) {}
+
+CIRGenAction::~CIRGenAction() { mlirModule.release(); }
+
+std::unique_ptr
+CIRGenAction::CreateASTConsumer(CompilerInstance , StringRef inputFile) {
+ auto out = ci.takeOutputStream();
+
+ auto Result = std::make_unique(
+ action, ci.getDiagnostics(), (),
+ ci.getHeaderSearchOpts(), ci.getCodeGenOpts(), ci.getTargetOpts(),
+ ci.getLangOpts(), ci.getFrontendOpts(), std::move(out));
+ cgConsumer = Result.get();
+
+ return std::move(Result);
ChuanqiXu9 wrote:
It may be pessimizing move
```suggestion
return Result;
```
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,59 @@
+//===- CIRGenerator.h - CIR Generation from Clang AST
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file declares a simple interface to perform CIR generation from Clang
+// AST
+//
+//===--===//
+
+#ifndef CLANG_CIRGENERATOR_H_
+#define CLANG_CIRGENERATOR_H_
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/DeclGroup.h"
+#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Diagnostic.h"
+
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/Support/VirtualFileSystem.h"
+
+#include
+
+namespace mlir {
+class MLIRContext;
+} // namespace mlir
+namespace cir {
+class CIRGenModule;
+
+class CIRGenerator : public clang::ASTConsumer {
+ virtual void anchor();
+ clang::DiagnosticsEngine
+ clang::ASTContext *astCtx;
+ llvm::IntrusiveRefCntPtr
+ fs; // Only used for debug info.
+
+ const clang::CodeGenOptions codeGenOpts; // Intentionally copied in.
ChuanqiXu9 wrote:
It may be helpful to add a comment to explain why this is intentionally copied
in?
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,61 @@ +//=== CIRGenAction.h - CIR Code Generation Frontend Action -*- C++ -*--===// ChuanqiXu9 wrote: Should we move this header to `CIR` or `FrontendAction`? Currently it lives in `CIRFrontendAction` but its implementation file lives in `FrontendAction` https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -42,6 +47,14 @@ CreateFrontendBaseAction(CompilerInstance ) {
StringRef Action("unknown");
(void)Action;
+ auto UseCIR = CI.getFrontendOpts().UseClangIRPipeline;
+ auto Act = CI.getFrontendOpts().ProgramAction;
+ auto EmitsCIR = Act == EmitCIR;
+
+ if (!UseCIR && EmitsCIR)
+llvm::report_fatal_error(
+"-emit-cir and -emit-cir-only only valid when using -fclangir");
ChuanqiXu9 wrote:
What is `-emit-cir-only`? Should we remove that?
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,88 @@
+//===--- CIRGenAction.cpp - LLVM Code generation Frontend Action -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/CIRFrontendAction/CIRGenAction.h"
+#include "clang/CIR/CIRGenerator.h"
+#include "clang/Frontend/CompilerInstance.h"
+
+#include "mlir/IR/MLIRContext.h"
+#include "mlir/IR/OwningOpRef.h"
+
+using namespace cir;
+using namespace clang;
+
+namespace cir {
+
+class CIRGenConsumer : public clang::ASTConsumer {
+
+ virtual void anchor();
+
+ [[maybe_unused]] CIRGenAction::OutputType action;
+
+ [[maybe_unused]] DiagnosticsEngine
+ [[maybe_unused]] const HeaderSearchOptions
+ [[maybe_unused]] const CodeGenOptions
+ [[maybe_unused]] const TargetOptions
+ [[maybe_unused]] const LangOptions
+ [[maybe_unused]] const FrontendOptions
+
+ std::unique_ptr outputStream;
+
+ [[maybe_unused]] ASTContext *astContext{nullptr};
+ IntrusiveRefCntPtr FS;
+ std::unique_ptr gen;
+
+public:
+ CIRGenConsumer(CIRGenAction::OutputType action,
+ DiagnosticsEngine ,
+ IntrusiveRefCntPtr VFS,
+ const HeaderSearchOptions ,
+ const CodeGenOptions ,
+ const TargetOptions ,
+ const LangOptions ,
+ const FrontendOptions ,
+ std::unique_ptr os)
+ : action(action), diagnosticsEngine(diagnosticsEngine),
+headerSearchOptions(headerSearchOptions),
+codeGenOptions(codeGenOptions), targetOptions(targetOptions),
+langOptions(langOptions), feOptions(feOptions),
+outputStream(std::move(os)), FS(VFS),
+gen(std::make_unique(diagnosticsEngine, std::move(VFS),
+ codeGenOptions)) {}
+
+ bool HandleTopLevelDecl(DeclGroupRef D) override {
+gen->HandleTopLevelDecl(D);
+return true;
+ }
+};
+} // namespace cir
+
+void CIRGenConsumer::anchor() {}
+
+CIRGenAction::CIRGenAction(OutputType act, mlir::MLIRContext *mlirContext)
+: mlirContext(mlirContext ? mlirContext : new mlir::MLIRContext),
+ action(act) {}
+
+CIRGenAction::~CIRGenAction() { mlirModule.release(); }
+
+std::unique_ptr
+CIRGenAction::CreateASTConsumer(CompilerInstance , StringRef inputFile) {
+ auto out = ci.takeOutputStream();
+
+ auto Result = std::make_unique(
+ action, ci.getDiagnostics(), (),
+ ci.getHeaderSearchOpts(), ci.getCodeGenOpts(), ci.getTargetOpts(),
+ ci.getLangOpts(), ci.getFrontendOpts(), std::move(out));
+ cgConsumer = Result.get();
ChuanqiXu9 wrote:
If I read correctly, `cgConsumer` is only used here? I guess it is needed in
following patches. But slightly odd.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
https://github.com/ChuanqiXu9 commented: BTW, it will be helpful to create subscribing team to help people to get informed in time. (I just saw the patch accidently.) https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
bcardosolopes wrote:
Thanks @AaronBallman
> But again, the important thing is to at least be self-consistent. e.g.,
It's a good point to reinforce, updated.
> I just don't think it benefits anyone to do so other than to make it easier
> to land the initial patches, which doesn't seem like a compelling reason to
> stick with auto.
Fair. I moved the usage of `auto` for `mlir::Location` for things outside
"CIRGen/FrontendAction" but allowed as part of `lib/CIR/{Transforms,*}`, where
the context makes it way more easier to understand (e.g. it can only be one
thing, no Clang's source locations expected at that point)
> I think that's something we can live with, but like above, this makes it
> harder for the community in general while making it easier for folks already
> involved with MLIR.
Ok, I'll take the "we can live with" for this one.
Doc updated accordingly:
https://llvm.github.io/clangir/GettingStarted/coding-guideline.html
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
AaronBallman wrote: > > I have no idea whether that returns a `SourceLocation`, a `PresumedLoc`, a > > `FullSourceLoc`, etc, so I don't think that is a use of `auto` I would want > > to have to reason about as a reviewer. That said, if the only use of `loc` > > is: `Diag(loc, diag::err_something);` then the type really doesn't matter > > and use of `auto` is probably fine. It's a judgment call. > > For location, `getLoc` always return `mlir::Location`, it's not related to > Clang's source location. CIRGen use of `loc` is mostly to pass it down to > MLIR APIs, because MLIR source locations are required whenever one is > building or rewriting an operation. We usually convert Clang's location into > MLIR and pass those around as expected. Has that changed your opinion for > this specific case? My preference is still to spell it out; I would have had no idea that there was an `mlir::Location` type and presumed this was using Clang's source location types. I can see the argument either way, but I prefer we optimize for readbility of the code by people *not* ensconced in MLIR. Otherwise this becomes harder for anyone other than MLIR folks to get involved with. That said, if it's inside of the `lib` directory and isn't part of a public interface, it's probably not the end of the world to stick with `auto`. I just don't think it benefits anyone to do so other than to make it easier to land the initial patches, which doesn't seem like a compelling reason to stick with `auto`. > > Again, I have no idea what the type of that is and I have no way to verify > > that it actually _uses_ value semantics because the pointer and qualifiers > > can be inferred and references can be dropped. That makes it harder to > > review the code; I would spell out the type in this case as well. > > Makes sense. For some background: MLIR "instructions", usually called > "operations", usually have pretty default return types when querying > operands, `mlir::Value` for SSA values or when an `mlir::Attribute` is > returned, the getter/setter is suffixed with `Attr`. Would it be reasonable > to suggest a guideline where in CIRGen we get more strict on not using `auto` > but for other passes and transformations it should be fine to use auto on > those? My rationale here is that post CIRGen it's very likely the reviwer > audience is going to be more among MLIR experts than Clang experts and the > former are more familiar with the idiom. I think that's something we can live with, but like above, this makes it harder for the community in general while making it easier for folks already involved with MLIR. > > My thinking is: 1) (most important) the convention should be consistent > > with other nearby code, whatever that convention happens to be. > > Ok, for camelBack, it seems like `clang/lib/CodeGen` is already adopting it > in multiple places (unsure if it's intentional or not?), and looks incomplete > because probably no one took the effort to make it uniform? The signal this > gives me is that we should just do it (use camelBack) for CIRGen. That seems reasonable to me. > Yep, thank you @AaronBallman for putting the time here to help us set the > right approach looking fwd. To be more specific, I'd suggest: > > - `clang/include` -> LLVM > - `lib/Driver/*` -> LLVM > - `lib/FrontendTool` -> LLVM > - `lib/CIR/FrontendAction` -> LLVM > - `lib/CIR/CodeGen` -> MLIR > - `lib/CIR/*` -> MLIR > > What do you think? I think that's a reasonable compromise. @erichkeane WDYT? https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
AaronBallman wrote: > What part of the "camelBack" is visible to the user of the API? (we're not > talking about method names or class names here I believe) Parameter names, which show up in tooltips in IDEs, etc. But again, the important thing is to at least be self-consistent. e.g., ``` CIRGenModule(mlir::MLIRContext , clang::ASTContext , const clang::CodeGenOptions , clang::DiagnosticsEngine ); ``` should be: ``` CIRGenModule(mlir::MLIRContext , clang::ASTContext , const clang::CodeGenOptions , clang::DiagnosticsEngine ); ``` or ``` CIRGenModule(mlir::MLIRContext , clang::ASTContext , const clang::CodeGenOptions clang::DiagnosticsEngine ); ``` rather than in a mixed form as it currently is. https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
bcardosolopes wrote: I encoded the discussion in our clangir.org page, with some speculation on Aaron's pending replies, happy to change and fix anything. In the future we'll upstream that too, but for the time being it's here: https://llvm.github.io/clangir/GettingStarted/coding-guideline.html https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
joker-eph wrote: What part of the "camelBack" is visible to the user of the API? (and in general the method prototype in the header should likely use the same parameter names as the implementation file) https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
bcardosolopes wrote: > What is obvious in the MLIR space is not necessarily what's obvious in Clang; Sure. > I have no idea whether that returns a `SourceLocation`, a `PresumedLoc`, a > `FullSourceLoc`, etc, so I don't think that is a use of `auto` I would want > to have to reason about as a reviewer. That said, if the only use of `loc` > is: `Diag(loc, diag::err_something);` then the type really doesn't matter and > use of `auto` is probably fine. It's a judgment call. For location, `getLoc` always return `mlir::Location`, it's not related to Clang's source location. CIRGen use of `loc` is mostly to pass it down to MLIR APIs, because MLIR source locations are required whenever one is building or rewriting an operation. We usually convert Clang's location into MLIR and pass those around as expected. Has that changed your opinion for this specific case? > Again, I have no idea what the type of that is and I have no way to verify > that it actually _uses_ value semantics because the pointer and qualifiers > can be inferred and references can be dropped. That makes it harder to review > the code; I would spell out the type in this case as well. Makes sense. For some background: MLIR "instructions", usually called "operations", usually have pretty default return types when querying operands, `mlir::Value` for SSA values or when an `mlir::Attribute` is returned, the getter/setter is suffixed with `Attr`. Would it be reasonable to suggest a guideline where in CIRGen we get more strict on not using `auto` but for other passes and transformations it should be fine to use auto on those? My rationale here is that post CIRGen it's very likely the reviwer audience is going to be more among MLIR experts than Clang experts and the former are more familiar with the idiom. > The big things for reviewers are: don't use `auto` if the type isn't > incredibly obvious (spelled in the initialization or extremely obvious based > on immediately surrounding code) and don't make us infer important parts the > declarator (if it's a `const Foo *` and `auto` makes sense for some reason, > spell it `const auto *` rather than `auto`). Agreed. Btw, I'm not advocating for plain `auto` here/everywhere. This is more about `auto` usage big picture. > And if a reviewer asks for something to be converted from `auto` to an > explicit type name, assume that means the code isn't as readable as it could > be and switch to a concrete type (we should not be arguing "I think this is > clear therefore you should also think it's clear" during review, that just > makes the review process painful for everyone involved). Agreed. If it makes the reviewer job easier we should just abide. > My thinking is: 1) (most important) the convention should be consistent with > other nearby code, whatever that convention happens to be. Ok, for camelBack, it seems like `clang/lib/CodeGen` is already adopting it in multiple places (unsure if it's intentional or not?), and looks incomplete because probably no one took the effort to make it uniform? The signal this gives me is that we should just do it (use camelBack) for CIRGen. 2) if there's not already a convention to follow and if it's code that lives in `clang/include/clang`, it should generally follow the LLVM style (it's an "external" interface) because those are the APIs that folks outside of CIR will be calling into. If it's code that lives in `clang/lib/`, following the MLIR style is less of a concern. Great & agreed, thanks for the clarification. > ... changing to the LLVM style is generally preferred over changing to the > MLIR style. One caveat to this is: avoid busywork but use your best judgement > on how we eventually get to a consistent use of the LLVM style. ... it's > probably best to just bite the bullet and switch to LLVM style even though > MLIR would be less effort. Does this all seem reasonable? Yep, thank you @AaronBallman for putting the time here to help us set the right approach looking fwd. To be more specific, I'd suggest: - `clang/include` -> LLVM - `lib/Driver/*` -> LLVM - `lib/FrontendTool` -> LLVM - `lib/CIR/FrontendAction` -> LLVM - `lib/CIR/CodeGen` -> MLIR - `lib/CIR/*` -> MLIR What do you think? https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
AaronBallman wrote: > Thanks for everyone's input so far. Let me try to summarize discussions in > this PR so we can set on an approach and give advice to our CIR community > (and encode on our webpage) on how to move forward in upcoming patches. > Useful resources: > > * [LLVM Coding Standard](https://llvm.org/docs/CodingStandards.html) > > * [MLIR Style > Guide](https://mlir.llvm.org/getting_started/DeveloperGuide/#style-guide) > > > CC'ing more people who might care: @seven-mile, @Lancern. > ## Use of `auto` > > As @joker-eph mentioned, MLIR isn't meant to differ from LLVM/Clang, though > they encode the differences as part of their guidelines. The `auto` > [guidance](https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable) > is still in favor of us keeping it for all `isa`, `dyn_cast` and `cast`, > which is used in CIR, so it probably doesn't affect most of what we currently > care about. Here's my suggestion for the entire `lib/CIR`: > > 1. Use it for function templates such as `isa`, `dyn_cast`, `cast`, > `create` and `rewriteOp.*` variants. Agreed -- anywhere the type is spelled out somewhere in the initializer is fine (so also `static_cast`, `getAs`, `new`, etc). > 2. Use it for things considered obvious/common in MLIR space, examples: > > > * `auto loc = op->getLoc()`. What is obvious in the MLIR space is not necessarily what's obvious in Clang; I have no idea whether that returns a `SourceLocation`, a `PresumedLoc`, a `FullSourceLoc`, etc, so I don't think that is a use of `auto` I would want to have to reason about as a reviewer. That said, if the only use of `loc` is: `Diag(loc, diag::err_something);` then the type really doesn't matter and use of `auto` is probably fine. It's a judgment call. > * Getting operands and results from operations (they obey Value > Semantics), e.g.: `DepthwiseConv2DNhwcHwcmOp op; ...; auto stride = > op.getStrides();` Again, I have no idea what the type of that is and I have no way to verify that it actually *uses* value semantics because the pointer and qualifiers can be inferred and references can be dropped. That makes it harder to review the code; I would spell out the type in this case as well. > * Other examples we are happy to provide upon reviewer feedback if it > makes sense. The big things for reviewers are: don't use `auto` if the type isn't incredibly obvious (spelled in the initialization or extremely obvious based on immediately surrounding code) and don't make us infer important parts the declarator (if it's a `const Foo *` and `auto` makes sense for some reason, spell it `const auto *` rather than `auto`). And if a reviewer asks for something to be converted from `auto` to an explicit type name, assume that means the code isn't as readable as it could be and switch to a concrete type (we should not be arguing "I think this is clear therefore you should also think it's clear" during review, that just makes the review process painful for everyone involved). > Using the logic above, all `auto`s in this current PR have to be changed > (since none apply). > ## Var naming: CamelCase vs camelBack > > From this discussion, seems like @AaronBallman and @erichkeane are fine with > us using camelBack and all the other differences from [MLIR Style > Guide](https://mlir.llvm.org/getting_started/DeveloperGuide/#style-guide) in > CIRGen and the rest of CIR. Is that right? This is based on the comment: > > > The mixed naming conventions in the header should be fixed (preference is > > to follow LLVM style if we're changing code around, but if the local > > preference is for MLIR, that's fine so long as it's consistent). > > However, @AaronBallman also wrote: > > > Also, the fact that the naming keeps being inconsistent is a pretty strong > > reason to change to use the LLVM naming convention, at least for external > > interfaces. > > Should we ignore this in light of your first comment? If not, can you clarify > what do you mean by external interfaces? Just want to make sure we get it > right looking fwd. My thinking is: 1) (most important) the convention should be consistent with other nearby code, whatever that convention happens to be. 2) if there's not already a convention to follow and if it's code that lives in `clang/include/clang`, it should generally follow the LLVM style (it's an "external" interface) because those are the APIs that folks outside of CIR will be calling into. If it's code that lives in `clang/lib/`, following the MLIR style is less of a concern. So in the case of this review, there's mixed styles being used in the PR and so something has to change. If we're changing something anyway, changing to the LLVM style is generally preferred over changing to the MLIR style. One caveat to this is: avoid busywork but use your best judgement on how we *eventually* get to a consistent use of the LLVM style. If
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -42,6 +47,14 @@ CreateFrontendBaseAction(CompilerInstance ) {
StringRef Action("unknown");
(void)Action;
+ auto UseCIR = CI.getFrontendOpts().UseClangIRPipeline;
AaronBallman wrote:
> Nit: technically the coding standard does not say that, I believe you're
> mentioning a sufficient condition, not a necessary one, see
> https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
>
> > Use auto if and only if it makes the code more readable or easier to
> > maintain.
Yup. FWIW, the rule of thumb we use in Clang is that "readable" means "type is
spelled out in the initialization somewhere or is otherwise painful to spell
but is contextually quite obvious (e.g., use of iterators)" + "any time the
code reviewer asks to switch away from `auto`"
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
https://github.com/MaskRay edited https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
https://github.com/MaskRay edited https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -2900,7 +2900,7 @@ defm clangir : BoolFOption<"clangir", PosFlag, NegFlag LLVM pipeline to compile">, BothFlags<[], [ClangOption, CC1Option], "">>; -def emit_cir : Flag<["-"], "emit-cir">, Visibility<[CC1Option]>, +def emit_cir : Flag<["-"], "emit-cir">, Visibility<[ClangOption, CC1Option]>, MaskRay wrote: > Notably, -emit-llvm-bc does the wrong thing in all cases. clang -Xclang > -emit-llvm-bc hello.c does the wrong thing and emits an executable. clang > -Xclang -emit-llvm -S hello.c also does the wrong thing and emits a file > named hello.s that is actually llvm bitcode. The description of #91140 might be useful. You need `-S` or `-c` to disable linking. Then `-c -Xclang -emit-cir` can be used to override the assumed action passed by clangDriver to cc1. https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
Lancern wrote: FWIW, the current practice uses CamelCase for CIRGen and camelBack for all other CIR stuff. Most code in CIRGen is directly ported from clang CodeGen and the code style is kept as-is, while other part of CIR is invented from scratch and we follow MLIR style guides. I'm not sure whether this is an acceptable scheme, but the naming style problems pointed out specifically in this PR (e.g. `langOpts`) should be resolved if this scheme is to be followed as they appear in CIRGen. https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
bcardosolopes wrote: Thanks for everyone's input so far. Let me try to summarize two discussions in this PR so we can set on an approach and give advice to our CIR community (and encode on our webpage) on how to move forward in upcoming patches. Useful resources: - [LLVM Coding Standard](https://llvm.org/docs/CodingStandards.html) - [MLIR Style Guide](https://mlir.llvm.org/getting_started/DeveloperGuide/#style-guide) CC'ing more people who might care: @seven-mile, @Lancern. ## Use of `auto` As @joker-eph mentioned, MLIR isn't meant to differ from LLVM/Clang, though they encode the differences as part of their guidelines. The `auto` [guidance](https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable) is still in favor of us keeping it for all `isa`, `dyn_cast` and `cast`, which is used in CIR, so it probably doesn't affect most of what we currently care about. Here's my suggestion for the entire `lib/CIR`: 1. Use it for function templates such as `isa`, `dyn_cast`, `cast`, `create` and `rewriteOp.*` variants. 2. Use it for things considered obvious/common in MLIR space, examples: - `auto loc = op->getLoc()`. - Getting operands and results from operations (they obey Value Semantics), e.g.: `DepthwiseConv2DNhwcHwcmOp op; ...; auto stride = op.getStrides();` - Other examples we are happy to provide upon reviewer feedback if it makes sense. Using the logic above, all `auto`s in this current PR have to be changed (since none apply). ## Namings: CamelCase vs camelBack >From this discussion, seems like @AaronBallman and @erichkeane are fine with >us using camelBack and all the other differences from [MLIR Style >Guide](https://mlir.llvm.org/getting_started/DeveloperGuide/#style-guide) in >CIRGen and the rest of CIR. Is that right? This is based on the comment: > The mixed naming conventions in the header should be fixed (preference is to > follow LLVM style if we're changing code around, but if the local preference > is for MLIR, that's fine so long as it's consistent). However, @AaronBallman also wrote: > Also, the fact that the naming keeps being inconsistent is a pretty strong > reason to change to use the LLVM naming convention, at least for external > interfaces. Should we ignore this in light of your first comment? If not, can you clarify what do you mean by external interfaces? Just want to make sure we get it right looking fwd. Does this makes sense? Thoughts? https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
https://github.com/joker-eph edited https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -42,6 +47,14 @@ CreateFrontendBaseAction(CompilerInstance ) {
StringRef Action("unknown");
(void)Action;
+ auto UseCIR = CI.getFrontendOpts().UseClangIRPipeline;
joker-eph wrote:
MLIR isn't meant to differ from LLVM/Clang.
> Coding standard doesn't allow use of 'auto' here, only when the actual type
> of the variable is spelled on the RHS side (see next 2 lines too).
Nit: technically the coding standard does not say that, I believe you're
mentioning a sufficient condition, not a necessary one, see
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
> Use auto if and only if it makes the code more readable or easier to
> maintain.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -42,6 +47,14 @@ CreateFrontendBaseAction(CompilerInstance ) {
StringRef Action("unknown");
(void)Action;
+ auto UseCIR = CI.getFrontendOpts().UseClangIRPipeline;
bcardosolopes wrote:
Right, it's a different discussion than naming. @joker-eph: what's the `auto`
policy for MLIR, is it different from LLVM/Clang? I'm also having trouble to
find the previous discussions, let me search a bit more.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,61 @@
+//===--- CIRGenModule.h - Per-Module state for CIR gen --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is the internal per-translation-unit state used for CIR translation.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H
+#define LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H
+
+#include "CIRGenTypeCache.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Diagnostic.h"
+
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/MLIRContext.h"
+
+using namespace clang;
+namespace cir {
+
+/// This class organizes the cross-function state that is used while generating
+/// CIR code.
+class CIRGenModule : public CIRGenTypeCache {
+ CIRGenModule(CIRGenModule &) = delete;
+ CIRGenModule =(CIRGenModule &) = delete;
+
+public:
+ CIRGenModule(mlir::MLIRContext , clang::ASTContext ,
+ const clang::CodeGenOptions ,
+ clang::DiagnosticsEngine );
+
+ ~CIRGenModule();
+
+private:
+ /// Hold Clang AST information.
+ clang::ASTContext
+
+ const clang::LangOptions
bcardosolopes wrote:
Agreed, we've been using a mix so we didn't had to go the extra length for
something the community could ask us to change as part of upstreaming, so
indeed this is the right time to make it happen.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,88 @@
+//===--- CIRGenAction.cpp - LLVM Code generation Frontend Action -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/CIRFrontendAction/CIRGenAction.h"
+#include "clang/CIR/CIRGenerator.h"
+#include "clang/Frontend/CompilerInstance.h"
+
+#include "mlir/IR/MLIRContext.h"
+#include "mlir/IR/OwningOpRef.h"
+
+using namespace cir;
+using namespace clang;
+
+namespace cir {
+
+class CIRGenConsumer : public clang::ASTConsumer {
+
+ virtual void anchor();
+
+ [[maybe_unused]] CIRGenAction::OutputType action;
bcardosolopes wrote:
To silence warnings for things we are about to introduce in follow up commits.
Sometimes we prefetch the skeleton in NFC fashion, in order to make
functionality changes have a smaller / more relevant diff. But we could surely
remove them too.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,88 @@
+//===--- CIRGenAction.cpp - LLVM Code generation Frontend Action -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/CIRFrontendAction/CIRGenAction.h"
+#include "clang/CIR/CIRGenerator.h"
+#include "clang/Frontend/CompilerInstance.h"
+
+#include "mlir/IR/MLIRContext.h"
+#include "mlir/IR/OwningOpRef.h"
+
+using namespace cir;
+using namespace clang;
+
+namespace cir {
+
+class CIRGenConsumer : public clang::ASTConsumer {
+
+ virtual void anchor();
+
+ [[maybe_unused]] CIRGenAction::OutputType action;
AaronBallman wrote:
Why is so much of the interface marked as maybe unused?
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,88 @@
+//===--- CIRGenAction.cpp - LLVM Code generation Frontend Action -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/CIRFrontendAction/CIRGenAction.h"
+#include "clang/CIR/CIRGenerator.h"
+#include "clang/Frontend/CompilerInstance.h"
+
+#include "mlir/IR/MLIRContext.h"
+#include "mlir/IR/OwningOpRef.h"
+
+using namespace cir;
+using namespace clang;
+
+namespace cir {
+
+class CIRGenConsumer : public clang::ASTConsumer {
+
+ virtual void anchor();
+
+ [[maybe_unused]] CIRGenAction::OutputType action;
+
+ [[maybe_unused]] DiagnosticsEngine
+ [[maybe_unused]] const HeaderSearchOptions
+ [[maybe_unused]] const CodeGenOptions
+ [[maybe_unused]] const TargetOptions
+ [[maybe_unused]] const LangOptions
+ [[maybe_unused]] const FrontendOptions
+
+ std::unique_ptr outputStream;
+
+ [[maybe_unused]] ASTContext *astContext{nullptr};
+ IntrusiveRefCntPtr FS;
AaronBallman wrote:
Naming inconsistency.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,59 @@ +//===- CIRGenerator.h - CIR Generation from Clang AST -===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// This file declares a simple interface to perform CIR generation from Clang +// AST +// +//===--===// + +#ifndef CLANG_CIRGENERATOR_H_ +#define CLANG_CIRGENERATOR_H_ + +#include "clang/AST/ASTConsumer.h" +#include "clang/AST/DeclGroup.h" AaronBallman wrote: This can be replaced with a forward declare. https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,59 @@ +//===- CIRGenerator.h - CIR Generation from Clang AST -===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// This file declares a simple interface to perform CIR generation from Clang +// AST +// +//===--===// + +#ifndef CLANG_CIRGENERATOR_H_ AaronBallman wrote: The usual pattern would be to name this `LLVM_CLANG_CIR_CIRGENERATOR_H` (`project_subproject_directory_headername_extension`) https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,46 @@
+//===--- CIRGenerator.cpp - Emit CIR from ASTs
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This builds an AST and converts it to CIR.
+//
+//===--===//
+
+#include "CIRGenModule.h"
+
+#include "clang/AST/DeclGroup.h"
+#include "clang/CIR/CIRGenerator.h"
+
+using namespace cir;
+using namespace clang;
+
+void CIRGenerator::anchor() {}
+
+CIRGenerator::CIRGenerator(clang::DiagnosticsEngine ,
+ llvm::IntrusiveRefCntPtr vfs,
+ const CodeGenOptions )
+: Diags(diags), fs(std::move(vfs)), codeGenOpts{CGO},
+ HandlingTopLevelDecls(0) {}
+CIRGenerator::~CIRGenerator() {}
+
+void CIRGenerator::Initialize(ASTContext ) {
+ using namespace llvm;
+
+ this->astCtx =
+
+ CGM = std::make_unique(*mlirCtx.get(), astCtx, codeGenOpts,
+ Diags);
+}
+
+bool CIRGenerator::HandleTopLevelDecl(DeclGroupRef D) {
+
+ for (DeclGroupRef::iterator I = D.begin(), E = D.end(); I != E; ++I) {
AaronBallman wrote:
`llvm::for_each`? range-based for loop?
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,61 @@ +//===--- CIRGenModule.h - Per-Module state for CIR gen --*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// This is the internal per-translation-unit state used for CIR translation. +// +//===--===// + +#ifndef LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H +#define LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H + +#include "CIRGenTypeCache.h" + +#include "clang/AST/ASTContext.h" +#include "clang/Basic/CodeGenOptions.h" +#include "clang/Basic/Diagnostic.h" AaronBallman wrote: All of these can be replaced with forward declares, right? https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,35 @@ +//===- CIRGenModule.cpp - Per-Module state for CIR generation -===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// This is the internal per-translation-unit state used for CIR translation. +// +//===--===// + +#include "CIRGenModule.h" + +#include "clang/AST/DeclBase.h" + +#include "llvm/Support/Debug.h" + +#include "mlir/IR/BuiltinOps.h" +#include "mlir/IR/Location.h" +#include "mlir/IR/MLIRContext.h" + +using namespace cir; +CIRGenModule::CIRGenModule(mlir::MLIRContext , AaronBallman wrote: More naming consistency issues to address. https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -42,6 +47,14 @@ CreateFrontendBaseAction(CompilerInstance ) {
StringRef Action("unknown");
(void)Action;
+ auto UseCIR = CI.getFrontendOpts().UseClangIRPipeline;
AaronBallman wrote:
Please follow the community conventions in this case because this is materially
different from naming. With naming, it's about style and while it's quite
distracting to have a unique style, it's doesn't really hurt understanding the
code. With use of `auto`, it's about being able to understand the code for
people not familiar with it; using `auto` in this way makes the code harder for
the community to maintain in ways that naming conventions don't really have.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,27 @@
+//===--- CIRGenTypeCache.h - Commonly used LLVM types and info -*- C++
--*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This structure provides a set of common types useful during CIR emission.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_CIR_CIRGENTYPECACHE_H
+#define LLVM_CLANG_LIB_CIR_CIRGENTYPECACHE_H
+
+namespace cir {
+
+/// This structure provides a set of types that are commonly used
+/// during IR emission. It's initialized once in CodeGenModule's
+/// constructor and then copied around into new CIRGenFunction's.
+struct CIRGenTypeCache {
+ CIRGenTypeCache() {}
AaronBallman wrote:
```suggestion
CIRGenTypeCache() = default;
```
Is the constructor even needed?
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,61 @@
+//===--- CIRGenModule.h - Per-Module state for CIR gen --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is the internal per-translation-unit state used for CIR translation.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H
+#define LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H
+
+#include "CIRGenTypeCache.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Diagnostic.h"
+
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/MLIRContext.h"
+
+using namespace clang;
+namespace cir {
+
+/// This class organizes the cross-function state that is used while generating
+/// CIR code.
+class CIRGenModule : public CIRGenTypeCache {
+ CIRGenModule(CIRGenModule &) = delete;
+ CIRGenModule =(CIRGenModule &) = delete;
+
+public:
+ CIRGenModule(mlir::MLIRContext , clang::ASTContext ,
+ const clang::CodeGenOptions ,
+ clang::DiagnosticsEngine );
+
+ ~CIRGenModule();
+
+private:
+ /// Hold Clang AST information.
+ clang::ASTContext
+
+ const clang::LangOptions
AaronBallman wrote:
FWIW, the fact that this naming concern keeps being raised is a red flag that
CIR should strongly consider addressing. Also, the fact that the naming keeps
being inconsistent is a pretty strong reason to change to use the LLVM naming
convention, at least for external interfaces.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,59 @@ +//===- CIRGenerator.h - CIR Generation from Clang AST -===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// This file declares a simple interface to perform CIR generation from Clang +// AST +// +//===--===// + +#ifndef CLANG_CIRGENERATOR_H_ +#define CLANG_CIRGENERATOR_H_ + +#include "clang/AST/ASTConsumer.h" +#include "clang/AST/DeclGroup.h" +#include "clang/Basic/CodeGenOptions.h" +#include "clang/Basic/Diagnostic.h" AaronBallman wrote: This can also be a forward declare. https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,59 @@
+//===- CIRGenerator.h - CIR Generation from Clang AST
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file declares a simple interface to perform CIR generation from Clang
+// AST
+//
+//===--===//
+
+#ifndef CLANG_CIRGENERATOR_H_
+#define CLANG_CIRGENERATOR_H_
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/DeclGroup.h"
+#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Diagnostic.h"
+
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/Support/VirtualFileSystem.h"
+
+#include
+
+namespace mlir {
+class MLIRContext;
+} // namespace mlir
+namespace cir {
+class CIRGenModule;
+
+class CIRGenerator : public clang::ASTConsumer {
+ virtual void anchor();
+ clang::DiagnosticsEngine
+ clang::ASTContext *astCtx;
+ llvm::IntrusiveRefCntPtr
AaronBallman wrote:
The mixed naming conventions in the header should be fixed (preference is to
follow LLVM style if we're changing code around, but if the local preference is
for MLIR, that's fine so long as it's consistent).
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,4 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s erichkeane wrote: > We don't have any IR to test just yet. @lanza you could use `FileCheck` with > `--allow-empty` here for now. You could also rename this test `emit-cir.c` > since the purpose for now is to not crash (as it would before this PR). Both of those are OK enough ideas to me. So at this point, we're just emitting a blank-file, right? Perhaps `--allow-empty` plus a `// CHECK-NOT: *` or something. https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -42,6 +47,14 @@ CreateFrontendBaseAction(CompilerInstance ) {
StringRef Action("unknown");
(void)Action;
+ auto UseCIR = CI.getFrontendOpts().UseClangIRPipeline;
erichkeane wrote:
Thats a question I think for @AaronBallman . This DOES live in the 'clang'
directory, so I would think this should all follow the Clang coding standards,
but I can see the justification for using the MLIR standard, same as the
capitalization (@lanza ).
There WAS some discussion elsewhere, but I cant find the thread, so I dont know
what was agreed upon.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,61 @@
+//=== CIRGenAction.h - CIR Code Generation Frontend Action -*- C++
-*--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_CIR_CIRGENACTION_H
+#define LLVM_CLANG_CIR_CIRGENACTION_H
+
+#include "clang/Frontend/FrontendAction.h"
+
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/OwningOpRef.h"
+
+namespace mlir {
+class MLIRContext;
+class ModuleOp;
+} // namespace mlir
+
+namespace cir {
+class CIRGenConsumer;
+
+class CIRGenAction : public clang::ASTFrontendAction {
+public:
+ enum class OutputType {
+EmitCIR,
erichkeane wrote:
Got it, thanks!
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
llvmbot wrote:
@llvm/pr-subscribers-clangir
Author: Nathan Lanza (lanza)
Changes
Build out the necessary infrastructure for the main entry point into
ClangIR generation -- CIRGenModule. A set of boilerplate classes exist
to facilitate this -- CIRGenerator, CIRGenAction, EmitCIRAction and
CIRGenConsumer. These all mirror the corresponding types from LLVM
generation by Clang's CodeGen.
The main entry point to CIR generation is
`CIRGenModule::buildTopLevelDecl`. It is currently just an empty
function. We've added a test to ensure that the pipeline reaches this
point and doesn't fail, but does nothing else. This will be removed in
one of the subsequent patches that'll add basic `cir.func` emission.
This patch also re-adds `-emit-cir` to the driver. lib/Driver/Driver.cpp
requires that a driver flag exists to facilirate the selection of the
right actions for the driver to create. Without a driver flag you get
the standard behaviors of `-S`, `-c`, etc. If we want to emit CIR IR
and, eventually, bytecode we'll need a driver flag to force this. This
is why `-emit-llvm` is a driver flag. Notably, `-emit-llvm-bc` as a cc1
flag doesn't ever do the right thing. Without a driver flag it is
incorrectly ignored and an executable is emitted. With `-S` a file named
`something.s` is emitted which actually contains bitcode.
---
Full diff: https://github.com/llvm/llvm-project/pull/91007.diff
16 Files Affected:
- (added) clang/include/clang/CIR/CIRGenerator.h (+59)
- (added) clang/include/clang/CIRFrontendAction/CIRGenAction.h (+61)
- (modified) clang/include/clang/Driver/Options.td (+1-1)
- (modified) clang/lib/CIR/CMakeLists.txt (+2)
- (added) clang/lib/CIR/CodeGen/CIRGenModule.cpp (+35)
- (added) clang/lib/CIR/CodeGen/CIRGenModule.h (+61)
- (added) clang/lib/CIR/CodeGen/CIRGenTypeCache.h (+27)
- (added) clang/lib/CIR/CodeGen/CIRGenerator.cpp (+46)
- (added) clang/lib/CIR/CodeGen/CMakeLists.txt (+23)
- (added) clang/lib/CIR/FrontendAction/CIRGenAction.cpp (+88)
- (added) clang/lib/CIR/FrontendAction/CMakeLists.txt (+17)
- (modified) clang/lib/Driver/ToolChains/Clang.cpp (+3)
- (modified) clang/lib/FrontendTool/CMakeLists.txt (+15)
- (modified) clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp (+18)
- (added) clang/test/CIR/hello.c (+4)
- (added) clang/test/CIR/lit.local.cfg (+2)
``diff
diff --git a/clang/include/clang/CIR/CIRGenerator.h
b/clang/include/clang/CIR/CIRGenerator.h
new file mode 100644
index 00..c9505d2473b437
--- /dev/null
+++ b/clang/include/clang/CIR/CIRGenerator.h
@@ -0,0 +1,59 @@
+//===- CIRGenerator.h - CIR Generation from Clang AST
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file declares a simple interface to perform CIR generation from Clang
+// AST
+//
+//===--===//
+
+#ifndef CLANG_CIRGENERATOR_H_
+#define CLANG_CIRGENERATOR_H_
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/DeclGroup.h"
+#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Diagnostic.h"
+
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/Support/VirtualFileSystem.h"
+
+#include
+
+namespace mlir {
+class MLIRContext;
+} // namespace mlir
+namespace cir {
+class CIRGenModule;
+
+class CIRGenerator : public clang::ASTConsumer {
+ virtual void anchor();
+ clang::DiagnosticsEngine
+ clang::ASTContext *astCtx;
+ llvm::IntrusiveRefCntPtr
+ fs; // Only used for debug info.
+
+ const clang::CodeGenOptions codeGenOpts; // Intentionally copied in.
+
+ [[maybe_unused]] unsigned HandlingTopLevelDecls;
+
+protected:
+ std::unique_ptr mlirCtx;
+ std::unique_ptr CGM;
+
+public:
+ CIRGenerator(clang::DiagnosticsEngine ,
+ llvm::IntrusiveRefCntPtr FS,
+ const clang::CodeGenOptions );
+ ~CIRGenerator();
+ void Initialize(clang::ASTContext ) override;
+ bool HandleTopLevelDecl(clang::DeclGroupRef D) override;
+};
+
+} // namespace cir
+
+#endif // CLANG_CIRGENERATOR_H_
diff --git a/clang/include/clang/CIRFrontendAction/CIRGenAction.h
b/clang/include/clang/CIRFrontendAction/CIRGenAction.h
new file mode 100644
index 00..aaf73ec2bffc47
--- /dev/null
+++ b/clang/include/clang/CIRFrontendAction/CIRGenAction.h
@@ -0,0 +1,61 @@
+//=== CIRGenAction.h - CIR Code Generation Frontend Action -*- C++
-*--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_CIR_CIRGENACTION_H
+#define
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,61 @@
+//=== CIRGenAction.h - CIR Code Generation Frontend Action -*- C++
-*--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_CIR_CIRGENACTION_H
+#define LLVM_CLANG_CIR_CIRGENACTION_H
+
+#include "clang/Frontend/FrontendAction.h"
+
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/OwningOpRef.h"
+
+namespace mlir {
+class MLIRContext;
+class ModuleOp;
+} // namespace mlir
+
+namespace cir {
+class CIRGenConsumer;
+
+class CIRGenAction : public clang::ASTFrontendAction {
+public:
+ enum class OutputType {
+EmitCIR,
bcardosolopes wrote:
This is a bit more complicated that it looks. The rationale is that we need to
duplicate all actions in order to use clangir transparently in all the
pipeline, see the current code here:
https://github.com/llvm/clangir/blob/main/clang/lib/CIR/FrontendAction/CIRGenAction.cpp
So we antecipate all existing available ones:
https://github.com/llvm/clangir/blob/f52e99e0bd566a4a3bfe73d1991d1ae692407d61/clang/include/clang/CIRFrontendAction/CIRGenAction.h#L33
If you have better ideas we're happy to abide.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
https://github.com/bcardosolopes edited https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,61 @@
+//===--- CIRGenModule.h - Per-Module state for CIR gen --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is the internal per-translation-unit state used for CIR translation.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H
+#define LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H
+
+#include "CIRGenTypeCache.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Diagnostic.h"
+
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/MLIRContext.h"
+
+using namespace clang;
+namespace cir {
+
+/// This class organizes the cross-function state that is used while generating
+/// CIR code.
+class CIRGenModule : public CIRGenTypeCache {
+ CIRGenModule(CIRGenModule &) = delete;
+ CIRGenModule =(CIRGenModule &) = delete;
+
+public:
+ CIRGenModule(mlir::MLIRContext , clang::ASTContext ,
+ const clang::CodeGenOptions ,
+ clang::DiagnosticsEngine );
+
+ ~CIRGenModule();
+
+private:
+ /// Hold Clang AST information.
+ clang::ASTContext
+
+ const clang::LangOptions
bcardosolopes wrote:
As long as we can keep this naming (e.g. `langOpts `) in `lib/CIR` and
`include/clang/CIR` should be fine for us.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -42,6 +47,14 @@ CreateFrontendBaseAction(CompilerInstance ) {
StringRef Action("unknown");
(void)Action;
+ auto UseCIR = CI.getFrontendOpts().UseClangIRPipeline;
bcardosolopes wrote:
Should this be the case for actual lib/CIR/CodeGen too? MLIR uses `auto`
extensively (perhaps because value semantics is common land there) and we have
been doing that as well for CIRGen and passes - it's going to be a massive
change for us, so we it'd be good to know if that should apply everywhere or
only into the areas of Clang isolated from MLIR.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,4 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s bcardosolopes wrote: We don't have any IR to test just yet. @lanza you could use `FileCheck` with `--allow-empty` here for now. You could also rename this test `emit-cir.c` since the purpose for now is to not crash (as it would before this PR). https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,4 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s erichkeane wrote: Would still like this to 'pipe' to filecheck. In addition to 'not crash', it would be worthwhile to ensure we emit sensible IR. https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -42,6 +47,14 @@ CreateFrontendBaseAction(CompilerInstance ) {
StringRef Action("unknown");
(void)Action;
+ auto UseCIR = CI.getFrontendOpts().UseClangIRPipeline;
erichkeane wrote:
Coding standard doesn't allow use of 'auto' here, only when the actual type of
the variable is spelled on the RHS side (see next 2 lines too).
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,61 @@
+//===--- CIRGenModule.h - Per-Module state for CIR gen --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is the internal per-translation-unit state used for CIR translation.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H
+#define LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H
+
+#include "CIRGenTypeCache.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Diagnostic.h"
+
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/MLIRContext.h"
+
+using namespace clang;
+namespace cir {
+
+/// This class organizes the cross-function state that is used while generating
+/// CIR code.
+class CIRGenModule : public CIRGenTypeCache {
+ CIRGenModule(CIRGenModule &) = delete;
+ CIRGenModule =(CIRGenModule &) = delete;
+
+public:
+ CIRGenModule(mlir::MLIRContext , clang::ASTContext ,
+ const clang::CodeGenOptions ,
+ clang::DiagnosticsEngine );
+
+ ~CIRGenModule();
+
+private:
+ /// Hold Clang AST information.
+ clang::ASTContext
+
+ const clang::LangOptions
erichkeane wrote:
What did we decide on the naming scheme in THIS directory? I figured with this
still being in clang it would use capital variable names? Was it THIS
directory (lib/CIR and include/clang/CIR) we agreed to earlier?
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,59 @@
+//===- CIRGenerator.h - CIR Generation from Clang AST
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file declares a simple interface to perform CIR generation from Clang
+// AST
+//
+//===--===//
+
+#ifndef CLANG_CIRGENERATOR_H_
+#define CLANG_CIRGENERATOR_H_
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/DeclGroup.h"
+#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Diagnostic.h"
+
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/Support/VirtualFileSystem.h"
+
+#include
+
+namespace mlir {
+class MLIRContext;
+} // namespace mlir
+namespace cir {
+class CIRGenModule;
+
+class CIRGenerator : public clang::ASTConsumer {
+ virtual void anchor();
+ clang::DiagnosticsEngine
+ clang::ASTContext *astCtx;
+ llvm::IntrusiveRefCntPtr
+ fs; // Only used for debug info.
+
+ const clang::CodeGenOptions codeGenOpts; // Intentionally copied in.
+
+ [[maybe_unused]] unsigned HandlingTopLevelDecls;
erichkeane wrote:
Probably better to bring this in only when used.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -53,8 +66,13 @@ CreateFrontendBaseAction(CompilerInstance ) {
case DumpTokens: return std::make_unique();
case EmitAssembly: return std::make_unique();
case EmitBC: return std::make_unique();
+#if CLANG_ENABLE_CIR
+ case EmitCIR:
erichkeane wrote:
So I'd like 1 of 2 things to happen here:
1- move the 'case' outside of the #if/#else
2- OR put both 'case' and 'make_unique' (and perhaps llvm_unreachable) on a
single line with the spacing to match the rest of the file.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,61 @@
+//===--- CIRGenModule.h - Per-Module state for CIR gen --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is the internal per-translation-unit state used for CIR translation.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H
+#define LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H
+
+#include "CIRGenTypeCache.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Diagnostic.h"
+
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/MLIRContext.h"
+
+using namespace clang;
+namespace cir {
+
+/// This class organizes the cross-function state that is used while generating
+/// CIR code.
+class CIRGenModule : public CIRGenTypeCache {
+ CIRGenModule(CIRGenModule &) = delete;
+ CIRGenModule =(CIRGenModule &) = delete;
+
+public:
+ CIRGenModule(mlir::MLIRContext , clang::ASTContext ,
+ const clang::CodeGenOptions ,
+ clang::DiagnosticsEngine );
+
+ ~CIRGenModule();
+
+private:
+ /// Hold Clang AST information.
+ clang::ASTContext
+
+ const clang::LangOptions
+
+ [[maybe_unused]] const clang::CodeGenOptions
+
+ /// A "module" matches a c/cpp source file: containing a list of functions.
+ mlir::ModuleOp theModule;
+
+ [[maybe_unused]] clang::DiagnosticsEngine
erichkeane wrote:
Same here, would be nice to only have to bring these in when necessary.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,61 @@
+//=== CIRGenAction.h - CIR Code Generation Frontend Action -*- C++
-*--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_CIR_CIRGENACTION_H
+#define LLVM_CLANG_CIR_CIRGENACTION_H
+
+#include "clang/Frontend/FrontendAction.h"
+
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/OwningOpRef.h"
+
+namespace mlir {
+class MLIRContext;
+class ModuleOp;
+} // namespace mlir
+
+namespace cir {
+class CIRGenConsumer;
+
+class CIRGenAction : public clang::ASTFrontendAction {
+public:
+ enum class OutputType {
+EmitCIR,
erichkeane wrote:
Whats the purpose here? Do we anticipate a 2nd output type?
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -2900,7 +2900,7 @@ defm clangir : BoolFOption<"clangir", PosFlag, NegFlag LLVM pipeline to compile">, BothFlags<[], [ClangOption, CC1Option], "">>; -def emit_cir : Flag<["-"], "emit-cir">, Visibility<[CC1Option]>, +def emit_cir : Flag<["-"], "emit-cir">, Visibility<[ClangOption, CC1Option]>, lanza wrote: @erichkeane @AaronBallman @MaskRay I had to re-add the ClangOption here. `lib/Driver/Driver.cpp` builds the `Action` framework out based on input to the driver. So to properly orchestrate the outputs you need a driver flag to tell it that, e.g., we are emitting a `.cir` textual output. e.g. `-fsyntax-only` exists to chop off the frontend at the right point and `-emit-llvm` seems intentionally to have been made a driver flag to orchestrate `.ll` or `.bc` output. Notably, `-emit-llvm-bc` does the wrong thing in all cases. `clang -Xclang -emit-llvm-bc hello.c` does the wrong thing and emits an executable. `clang -Xclang -emit-llvm -S hello.c` also does the wrong thing and emits a file named `hello.s` that is actually llvm bitcode. https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
https://github.com/lanza created https://github.com/llvm/llvm-project/pull/91007
Build out the necessary infrastructure for the main entry point into
ClangIR generation -- CIRGenModule. A set of boilerplate classes exist
to facilitate this -- CIRGenerator, CIRGenAction, EmitCIRAction and
CIRGenConsumer. These all mirror the corresponding types from LLVM
generation by Clang's CodeGen.
The main entry point to CIR generation is
`CIRGenModule::buildTopLevelDecl`. It is currently just an empty
function. We've added a test to ensure that the pipeline reaches this
point and doesn't fail, but does nothing else. This will be removed in
one of the subsequent patches that'll add basic `cir.func` emission.
This patch also re-adds `-emit-cir` to the driver. lib/Driver/Driver.cpp
requires that a driver flag exists to facilirate the selection of the
right actions for the driver to create. Without a driver flag you get
the standard behaviors of `-S`, `-c`, etc. If we want to emit CIR IR
and, eventually, bytecode we'll need a driver flag to force this. This
is why `-emit-llvm` is a driver flag. Notably, `-emit-llvm-bc` as a cc1
flag doesn't ever do the right thing. Without a driver flag it is
incorrectly ignored and an executable is emitted. With `-S` a file named
`something.s` is emitted which actually contains bitcode.
>From 17c81f79ede403e63010a39622d61937fcf898b4 Mon Sep 17 00:00:00 2001
From: Nathan Lanza
Date: Fri, 3 May 2024 20:19:45 +
Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?=
=?UTF-8?q?l=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.5
---
clang/include/clang/CIR/CIRGenerator.h| 59 +
.../clang/CIRFrontendAction/CIRGenAction.h| 61 +
clang/include/clang/Driver/Options.td | 2 +-
clang/lib/CIR/CMakeLists.txt | 2 +
clang/lib/CIR/CodeGen/CIRGenModule.cpp| 35
clang/lib/CIR/CodeGen/CIRGenModule.h | 61 +
clang/lib/CIR/CodeGen/CIRGenTypeCache.h | 27 ++
clang/lib/CIR/CodeGen/CIRGenerator.cpp| 46 ++
clang/lib/CIR/CodeGen/CMakeLists.txt | 23 +
clang/lib/CIR/FrontendAction/CIRGenAction.cpp | 88 +++
clang/lib/CIR/FrontendAction/CMakeLists.txt | 17
clang/lib/Driver/ToolChains/Clang.cpp | 3 +
clang/lib/FrontendTool/CMakeLists.txt | 15
.../ExecuteCompilerInvocation.cpp | 18
clang/test/CIR/hello.c| 4 +
clang/test/CIR/lit.local.cfg | 2 +
16 files changed, 462 insertions(+), 1 deletion(-)
create mode 100644 clang/include/clang/CIR/CIRGenerator.h
create mode 100644 clang/include/clang/CIRFrontendAction/CIRGenAction.h
create mode 100644 clang/lib/CIR/CodeGen/CIRGenModule.cpp
create mode 100644 clang/lib/CIR/CodeGen/CIRGenModule.h
create mode 100644 clang/lib/CIR/CodeGen/CIRGenTypeCache.h
create mode 100644 clang/lib/CIR/CodeGen/CIRGenerator.cpp
create mode 100644 clang/lib/CIR/CodeGen/CMakeLists.txt
create mode 100644 clang/lib/CIR/FrontendAction/CIRGenAction.cpp
create mode 100644 clang/lib/CIR/FrontendAction/CMakeLists.txt
create mode 100644 clang/test/CIR/hello.c
create mode 100644 clang/test/CIR/lit.local.cfg
diff --git a/clang/include/clang/CIR/CIRGenerator.h
b/clang/include/clang/CIR/CIRGenerator.h
new file mode 100644
index 00..c9505d2473b437
--- /dev/null
+++ b/clang/include/clang/CIR/CIRGenerator.h
@@ -0,0 +1,59 @@
+//===- CIRGenerator.h - CIR Generation from Clang AST
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file declares a simple interface to perform CIR generation from Clang
+// AST
+//
+//===--===//
+
+#ifndef CLANG_CIRGENERATOR_H_
+#define CLANG_CIRGENERATOR_H_
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/DeclGroup.h"
+#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Diagnostic.h"
+
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/Support/VirtualFileSystem.h"
+
+#include
+
+namespace mlir {
+class MLIRContext;
+} // namespace mlir
+namespace cir {
+class CIRGenModule;
+
+class CIRGenerator : public clang::ASTConsumer {
+ virtual void anchor();
+ clang::DiagnosticsEngine
+ clang::ASTContext *astCtx;
+ llvm::IntrusiveRefCntPtr
+ fs; // Only used for debug info.
+
+ const clang::CodeGenOptions codeGenOpts; // Intentionally copied in.
+
+ [[maybe_unused]] unsigned HandlingTopLevelDecls;
+
+protected:
+ std::unique_ptr mlirCtx;
+ std::unique_ptr CGM;
+
+public:
+ CIRGenerator(clang::DiagnosticsEngine ,
+
