[CIR][CIRGen][LowerToLLVM] Emit LLVM lifetime intrinsics #1554
+167,292
−5,231
Conversation
This file contains hidden or 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
Member
el-ev
commented
Apr 9, 2025
el-ev
commented
- Closes Emit LLVM lifetime intrinsics #1129.
…d neon_vaddvq_f64 (llvm#1238) [Neon intrinsic definition](https://developer.arm.com/architectures/instruction-sets/intrinsics/vaddv_f32). They are vector across operation which LLVM doesn't currently have a generic intrinsic about it. As a side note for brainstorm, it might be worth in the future for CIR to introduce Vector Across type operations even though LLVM dialect doesn't have it yet. This would help to expose opt opportunities. E.g. a very trivial constant fold can happen if we are adding across a constant vector.
…#1239) This implementation is different from OG in the sense we chose to use CIR op which eventually lowers to generic LLVM intrinsics instead of llvm.aarch64.neon intrinsics But down to the ASM level, [they are identical ](https://godbolt.org/z/Gbbos9z6Y).
…vm#1242) This patch follows llvm#1220 (comment) by augmenting `CIR_Type` with a new field, `tbaaName`. Specifically, it enables TableGen support for the `-gen-cir-tbaa-name-lowering` option, allowing for the generation of `getTBAAName` functions based on the `tbaaName`. This enhancement enables us to replace the hardcoded TBAA names in the `getTypeName` function with the newly generated `getTBAAName`.
This PR adds a bitcast when we rewrite globals type. Previously we just
set a new type and it worked.
But recently I started to test ClangIR with CSmith in order to find some
run time bugs and faced with the next problem.
```
typedef struct {
int x : 15;
uint8_t y;
} S;
S g = { -12, 254};
int main() {
printf("%d\n", g.y);
return 0;
}
```
The output for this program is ... 127 but not 254!
The reason is that first global var is created with the type of struct
`S`, then `get_member` operation is generated with index `1`
and then after, the type of the global is rewritten - I assume because
of the anon struct created on the right side in the initialization.
But the `get_member` operation still wants to access to the field at
index `1` and get a wrong byte.
If we change the `y` type to `int` we will fail on the verification
stage. But in the example above it's a run time error!
This is why I suggest to add a bitcast once we have to rewrite the
global type.
…2/u32, (llvm#1240) Co-authored-by: Bruno Cardoso Lopes <[email protected]>
We figure it would be nice to have a common place with all our known crashes that is tracked by git and is actively verified whether or not we can now support the crashes by lit. It can act as our source of truth for known failures and also being potential good first tasks for new developers. Add a simple test case of a known crash that involves copying a struct in a catch. Reviewers: smeenai, bcardosolopes Reviewed By: bcardosolopes Pull Request: llvm#1243
…#1247) Basically that is - the return value for `=` operator for bitfield assignment is wrong now. For example, the next function returns `7` for 3 bit bit field, though it should be `-1`: ``` int get_a(T *t) { return (t->a = 7); } ``` This PR fix it. Actually, the bug was in the lowering - the integer cast is applied in the wrong place (in comparison with the original codegen).
This PR changes changes the lowering of `cir.bool` to `i1` in both DorectToLLVM and ThroughMLIR. This dramatically simplifies the lowering logic of most operations and the lowered code itself as it naturally uses `i1` for anything boolean. The change involves separating between type lowering when scalars are involved and when memory is involved. This is a pattern that was inspired by clang's codegen which directly emits `i1` from the AST without intermediate higher level representation like CIR has. This also paves the way to more complex lowerings that are implemented in clang codegen through the three primitives added here: `Convert Type For Memory`, `Emit For Memory` and `Emit To Memory`. They are used in clang for non-trivial types like bitints but also extensible vectors.
Close llvm#1185 The patch itself seems slightly ad-hoc. As the issue tracked by llvm#1178, the fundamental solution may be to introduce two type systems to solve the inconsistent semantics for Union between LLVM IR and CIR. This will be great to handle other inconsistent semantics between LLVM IR and CIR if any. Back to the patch itself, although the code looks not good initially to me too. But I feel it may be a good workaround since clang/test/CIR/Lowering/union-array.c is an example for array of unions directly and clang/test/CIR/Lowering/nested-union-array.c gives an example for array of unions indirectly (array of structs which contain unions). So I feel we've already covered all the cases. And generally it should be good to use some simple and solid workaround before we introduce the formal full solution.
Lowering Neon `vsetq_lane_f64` References: [Clang CGBuiltin Implementation](https://github.com/llvm/clangir/blob/2b1a638ea07ca10c5727ea835bfbe17b881175cc/clang/lib/CodeGen/CGBuiltin.cpp#L12348) [Builtin definition](https://developer.arm.com/architectures/instruction-sets/intrinsics/vsetq_lane_f64)
…64` (llvm#1255) Lowering Neon `vaddd_s64` and `vaddd_u64` - [Clang CGBuiltin Implementation](https://github.com/llvm/clangir/blob/2b1a638ea07ca10c5727ea835bfbe17b881175cc/clang/lib/CodeGen/CGBuiltin.cpp#L12460-L12462) - [vaddd_s64 Builtin definition](https://developer.arm.com/architectures/instruction-sets/intrinsics/vaddd_s64) - [vaddd_u64 Builtin definition](https://developer.arm.com/architectures/instruction-sets/intrinsics/vaddd_u64)
…64` (llvm#1256) Lowering `neon_vsubd_s64` and `neon_vsubd_u64` - [Clang CGBuiltin Implementation](https://github.com/llvm/clangir/blob/2b1a638ea07ca10c5727ea835bfbe17b881175cc/clang/lib/CodeGen/CGBuiltin.cpp#L12463-L12465) - [vaddd_s64 Builtin definition](https://developer.arm.com/architectures/instruction-sets/intrinsics/vsubd_s64) - [vaddd_u64 Builtin definition](https://developer.arm.com/architectures/instruction-sets/intrinsics/vsubd_u64)
…16` and `vdivh_f16` (llvm#1258) Lowering: - `vaddh_f16` - `vsubh_f16` - `vmulh_f16` - `vdivh_f16`
…1259) Lowering `vshld_n_s64` and `vshld_n_u64`
Lowering `vshrd_n_s64`
Lowering Neon `vset_lane_f64` References: [Clang CGBuiltin Implementation](https://github.com/llvm/clangir/blob/2b1a638ea07ca10c5727ea835bfbe17b881175cc/clang/lib/CodeGen/CGBuiltin.cpp#L12342) [Builtin definition](https://developer.arm.com/architectures/instruction-sets/intrinsics/vset_lane_f64)
…llvm#1265) [Neon definiton](https://developer.arm.com/architectures/instruction-sets/intrinsics/#f:@navigationhierarchiessimdisa=[Neon]&q=vmaxv_s8) [OG implementation](https://github.com/llvm/clangir/blob/04d7dcfb2582753f3eccbf01ec900d60297cbf4b/clang/lib/CodeGen/CGBuiltin.cpp#L13202) Implementation in this PR is different from OG as 1. avoided code duplication by extracting out the common pattern 2. avoided using i32 as return type of the intrinsic call, so eliminated the need for casting result of the intrinsic call. This way of OG's implementation is quite unnecessary IMHO, this is MAX, not ADD or MUL. After all, using the expected type as return type of intrinsic call produces [the same ASM code](https://godbolt.org/z/3nKG7fxPb).
I continue to use `csmith` and catch run time bags. Now it's time to fix the layout for the const structs. There is a divergence between const structs generated by CIR and the original codegen. And this PR makes one more step to eliminate it. There are cases where the extra padding is required - and here is a fix for some of them. I did not write extra tests, since the fixes in the existing already covers the code I added. The point is that now the layout for all of these structs in the LLVM IR with and without CIR is the same.
Class `CIRGenFunction` contained three identical functions that converted from a Clang AST type (`clang::QualType`) to a ClangIR type (`mlir::Type`): `convertType`, `ConvertType`, and `getCIRType`. This embarrassment of duplication needed to be fixed, along with cleaning up other functions that convert from Clang types to ClangIR types. The three functions `CIRGenFunction::ConvertType`, `CIRGenFunction::convertType`, and `CIRGenFunction::getCIRType` were combined into a single function `CIRGenFunction::convertType`. Other functions were renamed as follows: - `CIRGenTypes::ConvertType` to `CIRGenTypes::convertType` - `CIRGenTypes::ConvertFunctionTypeInternal` to `CIRGenTypes::convertFunctionTypeInternal` - `CIRGenModule::getCIRType` to `CIRGenModule::convertType` - `ConstExprEmitter::ConvertType` to `ConstExprEmitter::convertType` - `ScalarExprEmitter::ConvertType` to `ScalarExprEmitter::convertType` Many cases of `getTypes().convertType(t)` and `getTypes().convertTypeForMem(t)` were changed to just `convertType(t)` and `convertTypeForMem(t)`, respectively, because the forwarding functions in `CIRGenModule` and `CIRGenFunction` make the explicit call to `getTypes()` unnecessary.
Reland previously reverted attempt now that this passes ASANified `ninja check-clang-cir`. Original message: We are missing cleanups all around, more incremental progress towards fixing that. This is supposed to be NFC intended, but we have to start changing some bits in order to properly match cleanup bits in OG. Start tagging places with more MissingFeatures to allow us to incrementally improve the situation.
…#1262) This patch adds support for the following GCC function attributes: - `__attribute__((const))` - `__attribute__((pure))` The side effect information is attached to the call operations during CIRGen. During LLVM lowering, these information is consumed to further emit appropriate LLVM metadata on LLVM call instructions.
…m#1249) C/C++ functions returning void had an explicit !cir.void return type while not having any returned value, which was breaking a lot of MLIR invariants when the CIR dialect is used in a greater context, for example with the inliner. Now, a C/C++ function returning void has no return type and no return values, which does not break the MLIR invariant about the same number of return types and returned values. This change does not keeps the same parsing/pretty-printed syntax as before for compatibility like in llvm#1203 because it requires some new features from the MLIR parser infrastructure itself, which is not great. This uses an optional type for function return type. The default MLIR parser for optional parameters requires an optional anchor we do not have in the syntax, so use a custom FuncType parser to handle the optional return type.
Adds implementation for ASinOp's lowering ThroughMLIR.
Lower vmaxv_u32
Lower vmaxvq_f64
Lower vmaxnmvq f32 and f64
In the review for upstreaming unary operation support (llvm/llvm-project#131369), it was suggested that the VisitPlus and VisitMinus functions should be combined. This is a backport of that refactoring.
This also removes some unused `#include`s. I choose to allow lowering to LLVM dialect when we're lowering CIR to MLIR core dialects, because some operations don't have their counterparts in these dialects (for example, `UnreachableOp -> llvm.unreachable` and `LLVMIntrinsicCallOp -> cir.llvm.intr.xxx`). I don't think we can delay them to the MLIR->LLVM pass as it seems we assume all CIR operations have been lowered after CIR->MLIR conversion. Co-authored-by: Yue Huang <[email protected]>
Lower vmaxnmv_f32
Update getZeroInitAttr to cover more FP types, Backport from (llvm/llvm-project#133100)
Adds implementation for TanOp's lowering via ThroughMLIR.
Upstream commit changed behavior llvm/llvm-project@ff585fe I tested in original clang and it produced the same IR with clangir. Related: llvm#1497
…nmvq_f32 (llvm#1546) ower vminnmv_f32, vminnmvq_f64 and vminnmvq_f32
…llvm#1547) When a pointer variable is initialized with a null pointer, the AST contains a NullToPointer cast. If we just emit a null pointer of the correct type, we will miss any side effects that occur in the initializer. This change adds code to emit the initializer expression if it is not a simple constant. This results in an extra null pointer constant being emitted when the expression has side effects, but the constant emitted while visiting the expression does not have the correct type, so the alternative would be to capture the emitted constant and bitcast it to the correct type. An extra constant seems less intrusive than an unnecessary bitcast.
Lower vminv_s32
Lower vminv_u32
llvm#1552) These entries exist in OG but are not present in CIR codegen.
Added: - `cos` - `floor` - `round` - `rint` - `nearbyint` - `sin` - `sqrt` - `tan` - `trunc`
Lower vminvq_f32 and vminvq_f64
During review of a patch for upstreaming the cir.struct type support, Erich Keane observed that the code we use for creating our type name for structures with templates was likely to be error prone. He recommended using QualType::print with the appropriate printing policy instead. This change does that. Erich also pointed out that RecordDecls always have a DeclContext so a few other lines could be eliminated where that was checked.
A lot of the unary math op lowerings follow the same template -- we can templatize this to remove redundant code and make things a little more neater. (Similar to what we do [here](https://github.com/llvm/clangir/blob/e4b8a48fb4d9a72a85e38f5439bcfb0673b4bea2/clang/lib/CIR/Lowering/ThroughMLIR/LowerCIRToMLIR.cpp#L502)) I've checked all existing LIT tests via `ninja clang-check-cir` and they seem to be passing fine.
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.