[CIR][ThroughMLIR] Support lowering SwitchOp without fallthrough to scf #986
+144,706
−185
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
Incremental step: we need the full try/catch to test `cir.try_call`, so testing coming next with the other necessary operations.
…and `cir.eh.inflight_exception` - Fix parser problems that were preventing testing and fix additional lowering missing for `cir.try_call`. - Add lowering from scratch for `cir.eh.inflight_exception`. End-to-end requires full exception support (still more lowering TBD to get there).
… unified AS (llvm#682) `TargetCodeGenInfo::getASTAllocaAddressSpace` is a bad design because it requires the targets return `LangAS`, which enforce the targets to consider which languages could be their upstream and unnecessarily complicate the target info. Unified AS in CIR is a better abstraction level for this kind of target info. Apart from that, the languages also has some requirements on the result address space of alloca. ```cpp void func() { int x; // Here, the AS of pointer `&x` is the alloca AS defined by the language. } ``` When we have inconsistency between the alloca AS defined by the language and the one from target info, we have to perform `addrspacecast` from the target one to the language one. This PR includes * New method `CGM.getLangTempAllocaAddressSpace` which explicitly specifies the alloca address space defined by languages. It replaces the vague `LangAS::Default` in the AS comparisons from OG CodeGen. * Replace `getASTAllocaAddressSpace` with `getCIRAllocaAddressSpace`, which returns CIR unified AS. * Also use `getCIRAllocaAddressSpace` as the result address space of `buildAlloca`. * Fix the lowering of `cir.alloca` operation to be address-space-aware. We don't perform any `addrspacecast` in this PR, i.e. all the related code paths still remain NYI and it's fine. That's because we don't even have a `(language, target)` pair holding the inconsistency. After these changes, in the previous OpenCL testcases we will see all the alloca pointers turning into private AS, without any `addrspacecast`.
…ering bugs. (llvm#756) In this pr, I lowering cir.do to scf.while, fix cir.while nested loop bugs and add test cases.
This PR adds CIRGen for scalar `co_yield` expressions.
This PR adds CIRGen support for the `__builtin_bit_cast` builtin. No new operations are added so the LLVM IR lowering is also added automatically.
…irect and Extend (llvm#763) This NFCI PR enhances the SPIR-V *CIRGen* ABI with Direct and Extend in both argument and return value, because some future test cases requires it. * kernel argument metadata needs arguments of promotable integer types * builtin functions like `get_global_id` returns `si64`, rather than void for all OpenCL kernels Given that CallConvLowering will replace these bits and other targets is already doing the same, I think it's safe to enable it now.
…nt ops (llvm#768) Soon I would like to submit a patch emitting OpenCL module metadata in LowerToLLVM path, which requires to attach the metadata to LLVM module when `amendOperaion` is called for MLIR module op. This PR refactors the method to a dispatcher to make the future changes cleaner.
Similar to llvm#705, this PR implements the remaining `genKernelArgMetadata()` logic. The attribute `cir.cl.kernel_arg_metadata` is also intentionally placed in the `cir.func`'s `extra_attrs` rather than `cir.func`'s standard `arg_attrs` list. Also, the metadata is stored by `Array` with proper verification on it. See the tablegen doc string for details. This is in order to * keep it side-by-side with `cl.kernel_metadata`. * still emit metadata when kernel has an *empty* arg list (see the test `kernel-arg-meatadata.cl`). * avoid horrors of repeating the long name `cir.cl.kernel_arg_metadata` for `numArgs` times. Because clangir doesn't support OpenCL built-in types and the `half` floating point type yet, their changes and test cases are not included. Corresponding missing feature flag is added.
…lvm#760) This PR simply adds the calling convention attribute to FuncOp with LLVM Lowering support. The overall approach follows `GlobalLinkageKind`: Extend the ODS, parser, printer and lowering pass. When the call conv is C call conv, it's omitted in the output assembly. --------- Co-authored-by: Bruno Cardoso Lopes <[email protected]>
…values (llvm#753) This commit makes the changes as following. 1. Enable array type GlobalOp lowering with initial values 2. Add error message when array size is not equal to initial string value size E.g. char big_string[10] = "abc";
…vm#769) There are two sources for the target allocation address space: one from `TargetCIRGenInfo::getCIRAllocaAddressSpace()` and another from `targetDataLayout.getAllocaMemorySpace()`. Since both are provided by the specific target, they should be consistent. This PR adds a check to ensure this consistency and avoid potential errors. The ctor of `CIRAllocaLowering` pattern is updated to pass the data layout in.
…e one in dialect (llvm#772) This PR remove the header `CIR/CodeGen/CallingConv.h` and migrates all `::cir::CallingConv` stuff to `::mlir::cir::CallingConv` in `CIRGenTypes` and `CIRGenFunctionInfo`. In TargetLowering library, LowerTypes and LowerFunctionInfo basically have the same clangCallConvToLLVMCallConv stuff. The CC there is the LLVM one. But those changes are not included because of the potential conflicts. We can still easily switch to the dialect when it's needed.
This PR adds OpenCL C language case to the enum `mlir::cir::SourceLanguage`, and maps `opts.OpenCL && !opts.OpenCLCPlusPlus` to it.
This PR introduce cir simplification pass. The idea is to have a pass
for the redundant operations removal/update.
Right now two pattern implemented, both related to the redundant `bool`
operations.
First pattern removes redundant casts from `bool` to `int` and back that
for some reasons appear in the code.
Second pattern removes sequential unary not operations (`!!`) .
For example, the code from the test is expanded from the simple check
that is common for C code:
```
#define CHECK_PTR(ptr) \
do { \
if (__builtin_expect((!!((ptr) == 0)), 0))\
return -42; \
} while(0)
```
I mark this PR as a draft for the following reasons:
1) I have no idea if it's useful for the community
2) There is a test fail - unfortunately current pattern rewriter run DCE
underneath the hood and looks like we can't workaround it.
It's enough just to add an operation to the list - in this case
`UnaryOp` - and call `applyOpPatternsAndFold`. I could rewrite a test a
little in order to make everything non dead or implement a simple fix
point algorithm for the particular task (I would do the former).
This PR adds support for complex cast operations. It adds the following new cast kind variants to the `cir.cast` operation: - `float_to_complex`, - `int_to_complex`, - `float_complex_to_real`, - `int_complex_to_real`, - `float_complex_to_bool`, - `int_complex_to_bool`, - `float_complex`, - `float_complex_to_int_complex`, - `int_complex`, and - `int_complex_to_float_complex`. CIRGen and LLVM IR support for these new cast variants are also included.
This PR adds folders for `cir.complex.create`, `cir.complex.real`, and `cir.complex.imag`. This PR adds a new attribute `#cir.complex` that represents a constant complex value. Besides, the CIR dialect does not have a constant materializer yet; this PR adds it. Address llvm#726 .
…all conv (llvm#778) This PR follows OG CodeGen to use SPIR ABI info whatever the target is when analysing the function info of SPIR-V kernels (identified by its calling convention). For example, when compiling OpenCL kernels to x86-64 target, the kernel should still use SPIR-V's ABIInfo. As we haven't implemented SPIR-V ABI handling for complex constructs, there should be no functional changes. There is a test for this logic in OG CodeGen: `clang/test/CodeGenOpenCL/kernels-have-spir-cc-by-default.cl`. It mainly involves structs, which is beyond the progress of CIR ABI stuff.
This PR adds the CIR address space attribute to GlobalOp and starts to resolve the missing feature flag `addressSpaceInGlobalVar`. The same asm format in pointer type is used: ``` cir.global external addrspace(offload_global) @addrspace1 = #cir.int<1> : !s32i ``` The parsing and printing helper is extracted into a common function to be reused by both `GlobalOp` and `PointerType` with two custom format proxy to it. That's because the signature of ODS generated method differs from the one for PointerType. Lowering to LLVM IR and CIRGen will come sequentially.
Incremental work: test is available in the followup commit.
This PR adds initial support for the `__int128` type. The `!cir.int` type is extended to support 128-bit integer types. This PR comes with a simple test that verifies the CIRGen and LLVM lowering of `!s128i` and `!u128i` work. Resolve llvm#953 .
…ulhq_lane and vqrdmulh_lane (llvm#985)
… pipeline This is causing lots of churn. `-fclangir-call-conv-lowering` is not mature enough, assumptions are leading to crashes we cannot track with special messages, leading to not great user experience. Turn this off until we have someone dedicated to roll this out.
While here add some bits for ptr auth and match OG.
…lvm#990) LLVM's verifier enforces this, which was previously causing us to fail verification. This is a bit of a band-aid; the overall linkage and visibility setting flow needs some work to match the original.
…lvm#965) As title, but important step in this PR is to allow CIR ShiftOp to take vector of int type as input type. As result, I added a verifier to ShiftOp with 2 constraints 1. Input type either all vector or int type. This is consistent with LLVM::ShlOp, vector shift amount is expected. 2. In the spirit of C99 6.5.7.3, shift amount type must be the same as result type, the if vector type is used. (This is enforced in LLVM lowering for scalar int type).
…iltinExpr (llvm#967) This PR helps us to triage unimplemented builtins (that are target independent). There are unhandled builtins in CIR Codegen `[CIRGenFunction::buildBuiltinExpr](https://github.com/llvm/clangir/blob/4c446b3287895879da598e23164d338d04bced3e/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp#L305)`. And those builtins have implementation in [OG](https://github.com/llvm/clangir/blob/4c446b3287895879da598e23164d338d04bced3e/clang/lib/CodeGen/CGBuiltin.cpp#L2573). Currently, those builtins just are treated as LibraryCall or some other ways which eventually get failure, and failure messages are confusing. This PR address this problem by refactoring `CIRGenFunction::buildBuiltinExpr` to keep the same skeleton as OG counterpart `CodeGenFunction::EmitBuiltinExpr`, and add builtin name to NYI message
Add more NFC skeleton while here.
Mochthon
force-pushed
the
cir_mlir_switch
branch
from
198ce40
to
7f0c7f4
Compare
|
I am refactoring the support for switch op and, from the process so far, I can send the patch this week. So does it make sense to suspend this PR for a while? |
|
Sent #1006 |
ChuanqiXu9
reviewed
Oct 25, 2024
| using mlir::OpConversionPattern<mlir::cir::SwitchOp>::OpConversionPattern; | ||
|
|
||
| mlir::LogicalResult | ||
| matchAndRewrite(mlir::cir::SwitchOp op, OpAdaptor adaptor, |
There was a problem hiding this comment.
BTW, I am confused why we need to lower SwitchOP? I think, we should deal with SwitchFlatOp only since all the SwitchOP should be faltten.
There was a problem hiding this comment.
Thank you for taking the time to review my code. Considering potential polyhedral optimizations, I kept the path using the -emit-cir option instead of -emit-cir-flat. The CIR generated this way provides more information and can be translated into higher-level MLIR dialects. This PR lowered SwitchOp because generating CIR by this option produces it.
bcardosolopes
requested changes
Oct 26, 2024
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.
This PR implemented lowering for
cir.switchinstances that terminate withcir.breaktoscf.index_switch.