[CIR][NFC] Move LoweringPrepare into CIRGen #1223
Open
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
as title. Also changed [neon-ldst.c](https://github.com/llvm/clangir/compare/main...ghehg:clangir-llvm-ghehg:macM3?expand=1#diff-ea4814b6503bff2b7bc4afc6400565e6e89e5785bfcda587dc8401d8de5d3a22) to make it have the same RUN options as OG [clang/test/CodeGen/aarch64-neon-intrinsics.c](https://github.com/llvm/clangir/blob/main/clang/test/CodeGen/aarch64-neon-intrinsics.c) Those options help us to avoid checking load/store pairs thus make the test less verbose and easier to compare against OG. Co-authored-by: Guojin He <[email protected]>
Implement derived-to-base address conversions for non-virtual base classes. The code gen for this situation was only implemented when the offset was zero, and it simply created a `cir.base_class_addr` op for which no lowering or other transformation existed. Conversion to a virtual base class is not yet implemented. Two new fields are added to the `cir.base_class_addr` operation: the byte offset of the necessary adjustment, and a boolean flag indicating whether the source operand may be null. The offset is easy to compute in the front end while the entire path of intermediate classes is still available. It would be difficult for the back end to recompute the offset. So it is best to store it in the operation. The null-pointer check is best done late in the lowering process. But whether or not the null-pointer check is needed is only known by the front end; the back end can't figure that out. So that flag needs to be stored in the operation. `CIRGenFunction::getAddressOfBaseClass` was largely rewritten. The code path no longer matches the equivalent function in the LLVM IR code gen, because the generated ClangIR is quite different from the generated LLVM IR. `cir.base_class_addr` is lowered to LLVM IR as a `getelementptr` operation. If a null-pointer check is needed, then that is wrapped in a `select` operation. When generating code for a constructor or destructor, an incorrect `cir.ptr_stride` op was used to convert the pointer to a base class. The code was assuming that the operand of `cir.ptr_stride` was measured in bytes; the operand is the number elements, not the number of bytes. So the base class constructor was being called on the wrong chunk of memory. Fix this by using a `cir.base_class_addr` op instead of `cir.ptr_stride` in this scenario. The use of `cir.ptr_stride` in `ApplyNonVirtualAndVirtualOffset` had the same problem. Continue using `cir.ptr_stride` here, but temporarily convert the pointer to type `char*` so the pointer is adjusted correctly. Adjust the expected results of three existing tests in response to these changes. Add two new tests, one code gen and one lowering, to cover the case where a base class is at a non-zero offset.
Fix #934 While here move scope op codegen outside the builder, so it's easier to dump blocks and operations while debugging.
After 5da4310, the LLVM dialect requires the variadic callee type to be present for variadic calls. The op builders take care of this automatically if you pass the function type, so change our lowering logic to do so. Add tests for this as well as a missing test for indirect function call lowering. Fixes #913 Fixes #933
See the test for the reproducer. It would crash due the NYI. See https://github.com/llvm/llvm-project/blob/327124ece7d59de56ca0f9faa2cd82af68c011b9/clang/lib/CodeGen/CGExpr.cpp#L1295-L1373, I found we've implemented all the cases in CGExpr.cpp. IIUC, I think we can remove the NYI.
Close #883. See the above issue for details
The title describes the purpose of the PR. The logic was gotten from the original CodeGen, and I added a test to check that `-fno-PIE` is indeed enabled.
This PR adds support for the `__fp16` type. CIRGen and LLVM lowering is included. Resolve #900 .
…pare_and_swap (#955) as title. Actually just follow the way in `makeBinaryAtomicValue` in the same file which did the right thing by creating SInt or UInt based on first argument's signess.
…for buildVarAnnotations
LLVM lowering support coming next.
…" (#944) This reverts commit 8f699fd and fixes some issues, namely: - CC lowering pass will no longer fail if the function has no AST information that won't be used. - Fixed CC lowering not disabling when running certain `cc1` compilation commands. - CC lowering can now be disabled when calling `cir-opt` and `cir-translate`. - Compilation commands that generate Object files should now invoke CC lowering by default.
I tried to run llvm-test-suite and turned out that there are many tests
fail with segfault due to old C style (let's remember Kernighan and
Ritchie) . This PR fix it by the usual copy-pasta from the original
codegen :)
So let's take a look at the code:
```
void foo(x) short x; {}
int main() {
foo(4);
return 0;
}
```
and CIR for `foo` function is:
```
cir.func @foo(%arg0: !s32i) {
%0 = cir.alloca !s16i, !cir.ptr<!s16i>, ["x", init]
%1 = cir.cast(bitcast, %0 : !cir.ptr<!s16i>), !cir.ptr<!s32i>
cir.store %arg0, %1 : !s32i, !cir.ptr<!s32i>
cir.return
}
```
We bitcast the **address** (!!!) and store a value of a bigger size
there.
And now everything looks fine:
```
cir.func no_proto @foo(%arg0: !s32i) {
%0 = cir.alloca !s16i, !cir.ptr<!s16i>, ["x", init]
%1 = cir.cast(integral, %arg0 : !s32i), !s16i
cir.store %1, %0 : !s16i, !cir.ptr<!s16i>
cir.return
}
```
We truncate an argument and store it.
P.S.
The `bitcast` that was there before looks a little bit suspicious and
dangerous. Are we sure we can do this unconditional cast while we create
`StoreOp` ?
This PR tries to give a simple initial implementation for eliminating redundant loads of constant objects, an idea originally posted by OfekShilon. Specifically, this PR adds a new unit attribute `const` to the `cir.alloca` operation. Presence of this attribute indicates that the alloca-ed object is declared `const` in the input source program. CIRGen is updated accordingly to start emitting this new attribute.
…nOp (#959) They should use PoisonOp (which becomes PoisonValue in LLVMIR) as it is the OG's choice. Proof: We generate VecCreateOp [here ](https://github.com/llvm/clangir/blob/2ca12fe5ec3a1e7279256f069010be2d68200585/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp#L1975) And it's OG counterpart is [here](https://github.com/llvm/clangir/blob/2ca12fe5ec3a1e7279256f069010be2d68200585/clang/lib/CodeGen/CGExprScalar.cpp#L2096) OG uses PoisonValue. As to VecSplatOp, OG unconditionally [chooses PoisonValue ](https://github.com/llvm/clangir/blob/2ca12fe5ec3a1e7279256f069010be2d68200585/llvm/lib/IR/IRBuilder.cpp#L1204) A even more solid proof for this case is that when we use OG to generate code for our test case I changed in this PR , its always using poison instead of undef as far as VecSplat and VecCreate is concerned. The [OG generated code for vectype-ext.cpp ](https://godbolt.org/z/eqx1rns86) here. The [OG generated code for vectype.cpp ](https://godbolt.org/z/frMjbKGeT) here. For reference, generated CIR for the test case vectype-ext.cpp is [here](https://godbolt.org/z/frMjbKGeT) This is to unblock #936 to help it set on the right path. Note: There might be other CIR vec ops that need to choose Poison to be consistent with OG, but I'd limit the scope of this PR, and wait to see issue pop up in the future.
as title. Base on my experience of [this type of test(https://github.com/llvm/clangir/blob/a7ac2b4e2055e169d9f556abf5821a1ccab666cd/clang/test/CIR/CodeGen/attribute-annotate-multiple.cpp#L51), The number of characters varies in this line as it's about full file path which changes during environment.
1. Add new `cir.vtt.address_point` op for visiting the element of VTT to initialize the virtual pointer. 2. Implement `getVirtualBaseClassOffset` method which provides a virtual offset to adjust to actual virtual pointers in virtual base. 3. Follows the original clang CodeGen scheme for the implementation of most other parts. @bcardosolopes's note: this is cherry-picked from an older PR from Jing Zhang and slightly modified for updates: applied review, test, doc and operation syntax. It does not yet has LLVM lowering support, I'm going to make incremental changes on top of this. Any necessary CIR modifications to this design should follow up shortly too. Also, to make this work I also added more logic to `addImplicitStructorParam`s` and `buildThisParam`.
as title. The generated code is the same as Clang codeden except in a small discrepancy when GEP: OG generates code like this: `%6 = getelementptr inbounds <4 x i16>, ptr %retval.i, i32 1` CIR generates a bit differently: `%6 = getelementptr <4 x i16>, ptr %retval.i, i64 1` Ptr offest might be trivial because choosing i64 over i32 as index type seems to be LLVM Dialect's choice. The lack of `inbounds` keyword might be an issue as `mlir::cir::PtrStrideOp` is currently not lowering to LLVM:GEPOp with `inbounds` attribute as `mlir::cir::PtrStrideOp` itself has no `inbounds`. It's probably because there was no need for it though we do have an implementation of [`CIRGenFunction::buildCheckedInBoundsGEP` ](https://github.com/llvm/clangir/blob/10d6f4b94da7e0181a070f0265d079419d96cf78/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp#L2762). Anyway, the issue is not in the scope of this PR and should be addressed in a separate PR. If we think this is an issue, I can create another PR and probably add optional attribute to `mlir::cir::PtrStrideOp` to achieve it. In addition to lowering work, a couple of more works: 1. Did a little refactoring on variable name changing into desired CamelBack case. 2. Changed neon-misc RUN Options to be consistent with other neon test files and make test case more concise.
as title. There are two highlights of the PR 1. The PR introduced a new test file to cover neon intrinsics that move data, which is a big category. This would the 5th neon test file. And we're committed to keep total number of neon test files within 6. This file uses another opt option instcombine, which makes test LLVM code more concise, and our -fclangir generated LLVM code would be identical to OG with this. It looks like OG did some instcombine optimization. 2. `getIntFromMLIRValue` helper function could be substituted by [`mlir::cir::IntAttr getConstOpIntAttr` in CIRGenAtomic.cpp](https://github.com/llvm/clangir/blob/24b24557c98d1c031572a567b658cfb6254f8a89/clang/lib/CIR/CodeGen/CIRGenAtomic.cpp#L337). The function `mlir::cir::IntAttr getConstOpIntAttr` is doing more than `getIntFromMLIRValue`, and there is FIXME in the comment, so not sure if we should just use `mlir::cir::IntAttr getConstOpIntAttr`, either is fine with me.
…ly (#961) Close #957 the previous algorithm to convert a multiple dimension array to a tensor is: fill the value one by one and fill the zero values in conditions. And it has some problems handling the multiple dimension array as above issue shows so that the generated values are not in the same shape with the original array. the new algorithm here is, full fill the values ahead of time with the correct element size and full fill the values to different slots and we only need to maintain the index to write. I feel the new version has better performance (avoid allocation) and better readability slightly.
…o llvm intrinsic (#960) This PR refactored Neon Built in code in clang/lib/CIR/CodeGen/CIRGenBuiltinAArch64.cpp a bit to make it cleaner. Also changed RUNOption of test file clang/test/CIR/CodeGen/AArch64/neon-arith.c to make test more concise, and easy to compare against OG (to compare, just remove -fclangir from llvm gen part of RUN, and the test should still pass)
Removes some NYIs. But left assert(false) due to missing tests. It looks better since it is not so scaring as NYI.
This PR adds support for base-to-derived and derived-to-base casts on pointer-to-data-member values. Related to #973.
#1194) Basically, for int type, the order of Ops is not the same as OG in the emitted LLVM IR. OG has constant as the second op position. See [OG's order ](https://godbolt.org/z/584jrWeYn).
Default assignment operator generation was failing because of memcpy generation for fields being unsupported. Implement it following CodeGen's example, as usual. Follow-ups will avoid emitting memcpys for fields of trivial class types, and extend this to copy constructors as well. Fixes #1128
Our previous logic here was matching CodeGen, which folds trivial assignment operator calls into memcpys, but we want to avoid that. Note that we still end up emitting memcpys for arrays of classes with trivial assignment operators; #1177 tracks fixing that.
CodeGen does so for trivial record types as well as non-record types; we only do it for non-record types.
This is a leftover from when ClangIR was initially focused on analysis and could ignore default method generation. We now handle default methods and should generate them in all cases. This fixes several bugs: - Default methods weren't emitted when emitting LLVM, only CIR. - Default methods only referenced by other default methods weren't emitted.
This PR updates the LLVM lowering part of load and stores to const allocas and makes use of the !invariant.group metadata in the result LLVM IR. The HoistAlloca pass is also updated. The const flag on a hoisted alloca is removed for now since their uses are not always invariants. Will update in later PRs to teach their invariants.
This PR adds CIRGen support for C++23 `[[assume(expr)]]` statement.
There's a lot that was missing here. Add NYIs and MissingFeatures as appropriate, so that we can start work on matching CodeGen.
ClangIR CodeGen makes use of both `mlir::MLIRContext` and `clang::ASTContext`. Referring to these as just "context" can be ambiguous. This change attempts to make the CodeGen code more clear by choosing better variable names. The change is almost entirely renaming: mostly change variables, parameters, and fields named `context` or `ctx` to `mlirContext` or `astContext`. There are some other renames having to do with contexts, and a small number of other drive-by fixes. (I considered also renaming functions named `getContext()`, but did not include that in this change.) This is entirely code cleanup. There should be no behavior changes.
This can't be simply implemented by our CIR Add via LLVM::AddOp, as i[t's saturated add.](https://godbolt.org/z/MxqGrj6fP)
Created using spr 1.3.5
…ch OG 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.
…n to match OG" Seems like windows bots are now broken! This reverts commit 9a63c50.
Created using spr 1.3.5
|
@bcardosolopes This was switched to only end up using the |
lanza
added a commit
that referenced
this pull request
Dec 12, 2024
Move LP into CIRGen and give it a handle on the CIRGenBuilderTy. Pull Request: #1223
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.
Move LP into CIRGen and give it a handle on the CIRGenBuilderTy.