[CIR] Check CLANG_ENABLE_CIR is actually enabled when using CIR
#888
+137,797
−86
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ZdlPV clang's choice of delete here changes from just hte pointer version to the sized version. Change the test to do the same
….vec.cmp to MLIR (llvm#694) This PR does Three things: 1. Fixes the BinOp lowering to MLIR issue where signed numbers were not handled correctly, and adds support for vector types. The corresponding test files have been modified. 2. Fixes the CmpOp lowering to MLIR issue where signed numbers were not handled correctly And modified test files. 3. Adds cir.vec.cmp lowering to MLIR along with the corresponding test files. I originally planned to complete the remaining cir.vec.* lowerings in this PR, but it seems there's quite a lot to do, so I'll split it into multiple PRs. --------- Co-authored-by: Kritoooo <[email protected]>
…perator (llvm#677) This PR is to fix the missing **nsw** flag in issue llvm#664 regarding add, mul arithmetic operations. there is also a problem with unary operations such as **Inc ,Dec,Plus,Minus and Not** . which should also have 'nsw' flag [example](https://godbolt.org/z/q3o3jsbe1). This part should need to be fixed through lowering.
This patch is a preparation for the AArch64 calling convention lowering. It adds the basic infrastructure to initialize the AArch64 ABI details and validates it against a trivial void return and argument call conv lowering.
as title. In this PR 1. make setDSOLocal an interface function. 2. implemented shouldAssumeDSOLocal function in CIRGenModule, using the same skeleton as shouldAssumeDSOLocal in OG's CodeGenModule.cpp. 3. added call sites of setDSOLocal within CIRGenModule, like what's in OG's CodeGenModule. 4. fixed printing format 5. LLVM lowering 6. keep CIRGenModule::setDSOLocal(mlir::Operation *Op) wrapper at call sites, so if we make changes to interface, we don't have to touch call sites since there are many. We don't have LLVM test for this PR yet, and it will be addressed by the next PR,: **TODO in the next PR:** 1. Implement setNonAliasAttributes in CIRGenModule.cpp, which should be called by CIRGenModule::buildGlobalFunctionDefinition. That way, we will set dso_local correctly for all func ops who have defs in the module. That way we should have LLVM test case in this next PR. detailed explanation below: Since LLVM asm printer omits dso_local in [isImplicitDSOLocal](https://github.com/llvm/clangir/blob/main/llvm/lib/IR/AsmWriter.cpp#L3689)(), and all we cover so far in CIR all fall into this category, we're not able to have a LLVM test. However, the case [isDeclarationForLinker()](https://github.com/llvm/clangir/blob/c28908396a3ba7bda6345907233e4f5c4e53a33e/clang/lib/CodeGen/CodeGenModule.cpp#L1655) should have a lot of test examples as all func defs should have dso_local, We don't have it CIR is because A to-do in our CG. When OG is building func def, after code is generated, it will call setDSOLocal again via setNonAliasAttributes—>SetCommonAttributes—>setGVProperties. The key difference is now GV is not declaration anymore. so satisfies the test if (!GV->isDeclarationForLinker()) return true; https://github.com/llvm/clangir/blob/f78f9a55e7cd6b9e350556e35097616676cf1f3e/clang/lib/CodeGen/CodeGenModule.cpp#L5864 But our CG missed this step of calling setNonAliasAttributes so it won’t give setDSOLocal another chance to get it right https://github.com/llvm/clangir/blob/c28908396a3ba7bda6345907233e4f5c4e53a33e/clang/lib/CIR/CodeGen/CIRGenModule.cpp#L496 **TODO in the next next PR** 2. add call to setDSOLocal in other parts of CG other than CIRGenModule. 3. implement DefaultVisibility check, didn't do in this PR as LLVM::DefaultVisibility has no direct counterpart in [MLIR::](mlir::SymbolTable::Visibility). Therefore, it takes care examination of cases to see what is the best emulation of hasDefaultVisibility in MLIR/CIR context as far as dsolocal is concerned. **TODO in future** other than DefaultVisibility check, we didn't implement canBenefitFromLocalAlias as it depends on other missing features like setComDat. There is a lot of cases we need to cover, so this is just the first step!
…vm#703) Mechanical rewrite to use the corresponding free functions; fixes llvm#702. I used a slightly modified version of the `clang-tidy` check provided in https://discourse.llvm.org/t/psa-deprecating-cast-isa-methods-in-some-classes/70909 to rewrite the C++ source files, regular expressions for the TableGen files, and manual cleanups where needed (e.g. chains like `x.foo().cast<A>().bar().cast<B>()`) I applied the following heuristic to determine which namespace prefix to use: - If the target type is not qualified, and the TU has `using namespace mlir` or the code is inside the `mlir` namespace -> use a plain `isa`/`cast`/... - Exception: Always qualify inside custom types and attributes, because their base classes define the very members we want to get rid of. - Else. i.e. the target type is qualified as `::mlir::` or `mlir::`, use that prefix. The `clang-tidy` check also rewrote `dyn_cast_or_null` to `dyn_cast_if_present`. I think that's useful because the former variant is going to be deprecated as well in the future. I'm using `-Werror=deprecated-declarations` to test the change (see 6b7420a); this required also changing two occurrences of `StringRef::equals` to `==`. I could also just drop the commit here; maybe we want to enable `-Werror` in general (there aren't too many other warnings left in the codebase). --------- Signed-off-by: Julian Oppermann <[email protected]>
) This PR implements the solution B as discussed in llvm#682. * Use the syntax `cir.ptr<T>` `cir.ptr<T, addrspace(target<0>)` `cir.ptr<T, addrspace(opencl_private)>` * Add a new `AddressSpaceAttr`, which is used as the new type of addrspace parameter in `PointerType` * `AddressSpaceAttr` itself takes one single `int64_t $value` as the parameter * TableGen templates to generate the conversion between `clang::LangAS -> int64_t $value <-> text-form CIR`
Adds the necessary bits to lower arguments and return values of type unsigned int for the x86_64 target. This includes adding logic for extend and direct argument-passing kinds.
Implement vector constants in ClangIR. Resolves issue llvm#498 - Add a `cir.const_vec`, simlar to `cir.const_array` and `cir.const_struct` Create a new kind of attribute, `cir::ConstVectorAttr` in the code or `#cir.const_vector` in the assembly, which represents a compile-time value of a `cir::VectorType`. The values for the elements within the vector are stored as attributes within an `mlir::ArrayAttr`. When doing CodeGen for a prvalue of vector type, try to represent it as `cir.const #cir.const_vector` first. If that fails, most likely because some of the elements are not compile-time values, fall back to the existing code that uses a `cir.vec.create` operation. When lowering directly to LLVM IR, lower `cir.const #cir.const_vector` as `llvm.mlir.constant(dense<[...]> : _type_) : _type_`. When lowering through other MLIR dialects, lower `cir.const #cir.const_vector` as `arith.constant dense<[...]> : _type_`. No new tests were added, but the expected results of the existing tests that use vector constants were updated.
This PR fixes a bug during the CIRGen of fp16 unary operations. Before this patch, for the expression `-x` where `x` is a fp16 value, CIRGen emits the code like the following: ```mlir %0 = cir.cast float_to_float %x : !cir.f16 -> !cir.float %1 = cir.cast float_to_float %0 : !cir.float -> !cir.f16 %2 = cir.unary minus %1 : !cir.fp16 ``` The expected CIRGen should instead be: ```mlir %0 = cir.cast float_to_float %x : !cir.f16 -> !cir.float %1 = cir.unary minus %0 : !cir.float %2 = cir.cast float_to_float %1 : !cir.float -> !cir.f16 ``` This PR fixes this issue.
Adds the necessary bits to lower arguments and return values of type unsigned int for the x86_64 target.
…#707) In this PR: as title we added setNonAliasAttributes in the skeleton of OG's setNonAliasAttributes, and call this function in buildGlobalFunctionDefinition after code for FuncOP is generated. This is needed for CIR OG to know FuncOP is not declaration anymore, thus giving shouldAssumeDsoLocal another run to make dso_local right. A couple of notes about test; 1. having to changed driver.c, because in terms of dso_local for func, masOS is different from other targets as even in OG, as [macOS is !isOSBinFormatELF()](https://github.com/llvm/clangir/blob/f78f9a55e7cd6b9e350556e35097616676cf1f3e/clang/lib/CodeGen/CodeGenModule.cpp#L1599), thus even OG doesn't set dso_local for its functions. 3. most of functions in existing tests still not getting dso_local in LLVM yet because they fall into case of [(RM != llvm::Reloc::Static && !LOpts.PIE) ](https://github.com/llvm/clangir/blob/f78f9a55e7cd6b9e350556e35097616676cf1f3e/clang/lib/CodeGen/CodeGenModule.cpp#L1605C6-L1605C47), which is more complicated to implement as we need to get canBenefitFromLocalAlias right. So I treated it as a missing feature and default it to false. We gonna leave it to another PR to address. In this PR, I just added additional test with -fpie option to my test so we get dso_local for functions without having to deal with this case. Next 2 PRs: PR1. call setNonAliasAttributes in buildGlobalVarDefinition, after initialization for GlobalOP is found, similar to FuncOp. didn't to it in this PR as there are many more test cases needed to be fixed/added for this case. PR2: try to implement canBenefitFromLocalAlias.
This PR implements the last piece to make CIR catch up upstream CodeGen
on `dynamic_cast` support. It ports an upstream optimization "exact
cast" to CIR.
The basic idea of exact cast is when `dynamic_cast` to a final class, we
don't have to call into the runtime -- we could just check if the
dynamic type of the source object is exactly the destination type by
quickly comparing the vtable pointers. To give a concrete example of
this optimization:
```cpp
struct Base { virtual ~Base(); };
struct Derived final : Base {};
Derived *test(Base *src) { return dynamic_cast<Derived *>(src); }
```
Without the optimization, we have to call the runtime function
`__dynamic_cast` to do the heavy and slow type check. After enabling the
optimization, we could quickly carry out the runtime type check by
inline checking whether the vtable ptr of `src` points to the vtable of
`Derived`.
This PR also fixes a bug in existing dynamic_cast CIRGen code. The bug
mistakenly removes the insertion point after emitting a call to
bad_cast, causing the CIRGen of any follow up statements to crash.
This PR adds LLVM lowering support for `_Float16` type. The only change we need to make here is adding a new type converter to the LLVM lowering pass. The majority of this PR is tests that check the generated LLVM IR. Later I'll add another separate PR that adds LLVM lowering support for the `__bf16` type. This PR is already big enough.
This PR adds LLVM lowering support for the `__bf16` type. To support its LLVM lowering, we just need to add a new type conversion rule to the LLVM lowering pass. The majority of this PR are the new LLVM IR checks in the tests.
Implements approach 2 in llvm#703 (comment), as discussed in the community call. Signed-off-by: Julian Oppermann <[email protected]>
This PR adds `!cir.complex` type to model the `_Complex` type in C. It also contains support for its CIRGen. In detail, this patch adds the following CIR types, ops, and attributes: - The `!cir.complex` type is added to model the `_Complex` type in C. This type is parameterized with the type of the components of the complex number, which must be either an integer type or a floating-point type. - ~The `#cir.complex` attribute is added to represent a literal value of `_Complex` type. It is a struct-like attribute that provides the real and imaginary part of the literal `_Complex` value.~ - ~The `#cir.imag` attribute is added to represent a purely imaginary number.~ - The `cir.complex.create` op is added to create a complex value from its real and imaginary parts. - ~The `cir.complex.real` and `cir.complex.imag` op is added to extract the real and imaginary part of a value of `!cir.complex` type, respectively.~ - The `cir.complex.real_ptr` and `cir.complex.imag_ptr` op is added to derive a pointer to the real and imaginary part of a value of `!cir.complex` type, respectively. CIRGen support for some of the fundamental complex number operations is also included. ~Note the implementation diverges from the original clang CodeGen, where expressions of complex types are handled differently from scalars and aggregates. Instead, this patch treats expressions of complex types as scalars, as such expressions can be simply lowered to a CIR value of `!cir.complex` type.~ This PR addresses llvm#445 .
There's no good reason to add our own switch here, given there's existing machinery to compute these names. Fallback to that instead.
LLVM support coming next.
… value (llvm#719) This commit fixes GlobalOp lowering for floating without initial value. It implies to be initialized with zeros. E.g. float f[100]; double d;
Mistakenly closed llvm#850 llvm#850 (review) This PR fixes array initialization for expression arguments. Consider the following code snippet `test.c`: ``` typedef struct { int a; int b[2]; } A; int bar() { return 42; } void foo() { A a = {bar(), {}}; } ``` When ran with `bin/clang test.c -Xclang -fclangir -Xclang -emit-cir -S -o -`, It produces the following error: ``` ~/clangir/clang/lib/CIR/CodeGen/CIRGenExprAgg.cpp:483: void {anonymous}::AggExprEmitter::buildArrayInit(cir::Address, mlir::cir::ArrayType, clang::QualType, clang::Expr*, llvm::ArrayRef<clang::Expr*>, clang::Expr*): Assertion `NumInitElements != 0' failed. ``` The error can be traced back to `CIRGenExprAgg.cpp`, and the fix is simple. It is possible to have an empty array initialization as an expression argument!
As title, if element type of vector type is sized, then the vector type should be deemed sized. This would enable us generate code for neon without triggering assertion
…eon_vrndaq_v (llvm#871) as title. This also added NeonType support for Float32 Co-authored-by: Guojin He <[email protected]>
…::saved_type::save
It will hit another assert when calling initFullExprCleanup.
This PR fixes the case, when a temporary var is used, and `alloca` operation is inserted in the block start before the `label` operation. Implementation: when we search for the `alloca` place in a block, we take label operations into account as well. Fix llvm#870 --------- Co-authored-by: Bruno Cardoso Lopes <[email protected]>
__attribute__((annotate()) was only accepting integer literals, preventing some meta-programming usage for example. This should be extended to some other kinds of types. --------- Co-authored-by: Bruno Cardoso Lopes <[email protected]>
Just as the title says, but only covers non-exception path, that's coming next.
Nothing unblocked yet, just hit next assert in the same path.
… exceptions Code path still hits an assert sooner, incremental NFC step.
…lvm#878) Close llvm#876 We've already considered the case that there are random stmt after a switch case: ``` for (auto *c : compoundStmt->body()) { if (auto *switchCase = dyn_cast<SwitchCase>(c)) { res = buildSwitchCase(*switchCase, condType, caseAttrs); } else if (lastCaseBlock) { // This means it's a random stmt following up a case, just // emit it as part of previous known case. mlir::OpBuilder::InsertionGuard guardCase(builder); builder.setInsertionPointToEnd(lastCaseBlock); res = buildStmt(c, /*useCurrentScope=*/!isa<CompoundStmt>(c)); } else { llvm_unreachable("statement doesn't belong to any case region, NYI"); } lastCaseBlock = builder.getBlock(); if (res.failed()) break; } ``` However, maybe this is an oversight, in the branch of ` if (lastCaseBlock)`, the insertion point will be updated automatically when the RAII object `guardCase` destroys, then we can assign the correct value for `lastCaseBlock` later. So we will see the weird code pattern in the issue side. BTW, I found the codes in CIRGenStmt.cpp are far more less similar with the ones other code gen places. Is this intentional? And what is the motivation and guide lines here?
…lvm#882) As title. Notice that for those intrinsics, just like OG, we do not lower to llvm intrinsics, instead, do vector insert. The test case is partially from OG [aarch64-neon-vget.c](https://github.com/llvm/clangir/blob/85bc6407f559221afebe08a60ed2b50bf1edf7fa/clang/test/CodeGen/aarch64-neon-vget.c) But, I did not do all signed and unsigned int tests because unsigned and signed of the same width essentially just use the same intrinsic ID thus exactly same code path as far as this PR concerns. --------- Co-authored-by: Guojin He <[email protected]>
Reviewers: bcardosolopes Reviewed By: bcardosolopes Pull Request: llvm#881
…#877) We need a target-independent way to distinguish OpenCL kernels in ClangIR. This PR adds a unit attribute `OpenCLKernelAttr` similar to the one in Clang AST. This attribute is attached to the extra attribute dictionary of `cir.func` operations only. (Not for `cir.call`.)
…'case' (llvm#879) Motivation example: ``` extern "C" void action1(); extern "C" void action2(); extern "C" void case_follow_label(int v) { switch (v) { case 1: label: case 2: action1(); break; default: action2(); goto label; } } ``` When we compile it, we will meet: ``` case Stmt::CaseStmtClass: case Stmt::DefaultStmtClass: assert(0 && "Should not get here, currently handled directly from SwitchStmt"); break; ``` in `buildStmt`. The cause is clear. We call `buildStmt` when we build the label stmt. To solve this, I think we should be able to build case stmt in buildStmt. But the new problem is, we need to pass the information like caseAttr and condType. So I tried to add such informations in CIRGenFunction as data member.
llvm#884) as title. This PR has simliar test case organization as to [PR882](llvm#882) Notice that comparing to OG, this PR combines cases for some pairs of intrinsics such as BI__builtin_neon_vget_lane_f32 and BI__builtin_neon_vdups_lane_f32. They have the same code generated in OG and CIRGen OG separate them into different case handling because it passes mnemonics which are different. CIRGen doesn't pass that so why not combine them. Co-authored-by: Guojin He <[email protected]>
as title, this would complete solution to fix issue [LLVM lowering missing comdat and constant attributes](llvm#801)
When the CMake option CLANG_ENABLE_CIR is *not* enabled the `-fclangir`
option was being silently ignored and LLVM IR would not be produced through
the ClangIR pipeline. This new check errors out in that condition,
preventing people from thinking CIR is enabled when it actually is not.
This also helps the `llvm_unreachable("CIR suppport not built into clang");`
statement to not actually be reached.
luismarques
changed the title
CLANG_ENABLE_CIR is actually enabled when using CIR
bcardosolopes
requested changes
Sep 26, 2024
| @@ -56,6 +56,12 @@ CreateFrontendBaseAction(CompilerInstance &CI) { | |||
| auto CIRAnalysisOnly = CI.getFrontendOpts().ClangIRAnalysisOnly; | |||
| auto EmitsCIR = Act == EmitCIR || Act == EmitCIRFlat || Act == EmitCIROnly; | |||
|
|
|||
| #if !CLANG_ENABLE_CIR | |||
There was a problem hiding this comment.
#if !defined(CLANG_ENABLE_CIR) should be a bit more safe.
For the error message, I suggest we already give some direction: ClangIR (CIR) support not enabled in the build, invoke cmake with -DCLANG_ENABLE_CIR=ON to enable, or something along those lines.
smeenai
force-pushed
the
main
branch
2 times, most recently
from
4aca8d4
to
a04cf10
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When the CMake option
CLANG_ENABLE_CIRis not enabled the-fclangiroption was being silently ignored and LLVM IR would not be produced through the ClangIR pipeline. This new check errors out in that condition, preventing people from thinking CIR is enabled when it actually is not.This also helps the
llvm_unreachable("CIR suppport not built into clang");statement below to not actually be reached.