[CIR][CodeGen][LowerToLLVM] Set calling convention for call ops #836
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
seven-mile
commented
Sep 13, 2024
Comment on lines
+887
to
+890
| auto newOp = rewriter.replaceOpWithNewOp<mlir::LLVM::InvokeOp>( | ||
| op, llvmResults, calleeAttr, callOperands, continueBlock, | ||
| mlir::ValueRange{}, landingPadBlock, mlir::ValueRange{}); | ||
| else | ||
| rewriter.replaceOpWithNewOp<mlir::LLVM::CallOp>(op, llvmResults, | ||
| calleeAttr, callOperands); | ||
| newOp.setCConv(cconv); |
There was a problem hiding this comment.
I don't have an idea on how to avoid 4 dups of saving newOp and setting CConv. Different from CIRCallOpInterface, LLVM uses CallOpInterface from MLIR std, which is not aware of calling convention.
If we don't consider hacky op->setAttr('calling_conv', ...), it seems we have to do a dynamic dispatch of at least size 2 (invoke + call) after rewriting. I chose to just leave it clear. It might be more appropriate to refactor this part after we have more similar logic like setCConv.
There was a problem hiding this comment.
I wouldn't worry too much about this. Since we don't need the result for anything else (yet), my suggestion would be:
rewriter.replaceOpWithNewOp<mlir::LLVM::InvokeOp>(
op, llvmResults, calleeAttr, callOperands, continueBlock,
mlir::ValueRange{}, landingPadBlock, mlir::ValueRange{}).setCConv(cconv);
| return builder.createCallOp(callLoc, directFuncOp, CIRCallArgs, | ||
| mlir::cir::CallingConv::C, extraFnAttrs); | ||
| } | ||
| return builder.createCallOp(callLoc, directFuncOp, CIRCallArgs, callingConv, |
There was a problem hiding this comment.
Tested by updated spir-calling-conv.cl.
bcardosolopes
approved these changes
Sep 13, 2024
Comment on lines
+887
to
+890
| auto newOp = rewriter.replaceOpWithNewOp<mlir::LLVM::InvokeOp>( | ||
| op, llvmResults, calleeAttr, callOperands, continueBlock, | ||
| mlir::ValueRange{}, landingPadBlock, mlir::ValueRange{}); | ||
| else | ||
| rewriter.replaceOpWithNewOp<mlir::LLVM::CallOp>(op, llvmResults, | ||
| calleeAttr, callOperands); | ||
| newOp.setCConv(cconv); |
There was a problem hiding this comment.
I wouldn't worry too much about this. Since we don't need the result for anything else (yet), my suggestion would be:
rewriter.replaceOpWithNewOp<mlir::LLVM::InvokeOp>(
op, llvmResults, calleeAttr, callOperands, continueBlock,
mlir::ValueRange{}, landingPadBlock, mlir::ValueRange{}).setCConv(cconv);
Hugobros3
pushed a commit
to shady-gang/clangir
that referenced
this pull request
Oct 2, 2024
…#836) This PR implements the CIRGen and Lowering part of calling convention attribute of `cir.call`-like operations. Here we have **4 kinds of operations**: (direct or indirect) x (`call` or `try_call`). According to our need and feasibility of constructing a test case, this PR includes: * For CIRGen, only direct `call`. Until now, the only extra calling conventions are SPIR ones, which cannot be set from source code manually using attributes. Meanwhile, OpenCL C *does not allow* function pointers or exceptions, therefore the only case remaining is direct call. * For Lowering, direct and indirect `call`, but not any `try_call`. Although it's possible to write all 4 kinds of calls with calling convention in ClangIR assembly, exceptions is quite hard to write and read. I prefer source-code-level test for it when it's available in the future. For example, possibly C++ `thiscall` with exceptions. * Extra: the verification of calling convention consistency for direct `call` and direct `try_call`. All unsupported cases are guarded by assertions or MLIR diags.
smeenai
pushed a commit
to smeenai/clangir
that referenced
this pull request
Oct 9, 2024
…#836) This PR implements the CIRGen and Lowering part of calling convention attribute of `cir.call`-like operations. Here we have **4 kinds of operations**: (direct or indirect) x (`call` or `try_call`). According to our need and feasibility of constructing a test case, this PR includes: * For CIRGen, only direct `call`. Until now, the only extra calling conventions are SPIR ones, which cannot be set from source code manually using attributes. Meanwhile, OpenCL C *does not allow* function pointers or exceptions, therefore the only case remaining is direct call. * For Lowering, direct and indirect `call`, but not any `try_call`. Although it's possible to write all 4 kinds of calls with calling convention in ClangIR assembly, exceptions is quite hard to write and read. I prefer source-code-level test for it when it's available in the future. For example, possibly C++ `thiscall` with exceptions. * Extra: the verification of calling convention consistency for direct `call` and direct `try_call`. All unsupported cases are guarded by assertions or MLIR diags.
smeenai
pushed a commit
to smeenai/clangir
that referenced
this pull request
Oct 9, 2024
…#836) This PR implements the CIRGen and Lowering part of calling convention attribute of `cir.call`-like operations. Here we have **4 kinds of operations**: (direct or indirect) x (`call` or `try_call`). According to our need and feasibility of constructing a test case, this PR includes: * For CIRGen, only direct `call`. Until now, the only extra calling conventions are SPIR ones, which cannot be set from source code manually using attributes. Meanwhile, OpenCL C *does not allow* function pointers or exceptions, therefore the only case remaining is direct call. * For Lowering, direct and indirect `call`, but not any `try_call`. Although it's possible to write all 4 kinds of calls with calling convention in ClangIR assembly, exceptions is quite hard to write and read. I prefer source-code-level test for it when it's available in the future. For example, possibly C++ `thiscall` with exceptions. * Extra: the verification of calling convention consistency for direct `call` and direct `try_call`. All unsupported cases are guarded by assertions or MLIR diags.
keryell
pushed a commit
to keryell/clangir
that referenced
this pull request
Oct 19, 2024
…#836) This PR implements the CIRGen and Lowering part of calling convention attribute of `cir.call`-like operations. Here we have **4 kinds of operations**: (direct or indirect) x (`call` or `try_call`). According to our need and feasibility of constructing a test case, this PR includes: * For CIRGen, only direct `call`. Until now, the only extra calling conventions are SPIR ones, which cannot be set from source code manually using attributes. Meanwhile, OpenCL C *does not allow* function pointers or exceptions, therefore the only case remaining is direct call. * For Lowering, direct and indirect `call`, but not any `try_call`. Although it's possible to write all 4 kinds of calls with calling convention in ClangIR assembly, exceptions is quite hard to write and read. I prefer source-code-level test for it when it's available in the future. For example, possibly C++ `thiscall` with exceptions. * Extra: the verification of calling convention consistency for direct `call` and direct `try_call`. All unsupported cases are guarded by assertions or MLIR diags.
lanza
pushed a commit
that referenced
this pull request
Nov 5, 2024
This PR implements the CIRGen and Lowering part of calling convention attribute of `cir.call`-like operations. Here we have **4 kinds of operations**: (direct or indirect) x (`call` or `try_call`). According to our need and feasibility of constructing a test case, this PR includes: * For CIRGen, only direct `call`. Until now, the only extra calling conventions are SPIR ones, which cannot be set from source code manually using attributes. Meanwhile, OpenCL C *does not allow* function pointers or exceptions, therefore the only case remaining is direct call. * For Lowering, direct and indirect `call`, but not any `try_call`. Although it's possible to write all 4 kinds of calls with calling convention in ClangIR assembly, exceptions is quite hard to write and read. I prefer source-code-level test for it when it's available in the future. For example, possibly C++ `thiscall` with exceptions. * Extra: the verification of calling convention consistency for direct `call` and direct `try_call`. All unsupported cases are guarded by assertions or MLIR diags.
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 implements the CIRGen and Lowering part of calling convention attribute of
cir.call-like operations. Here we have 4 kinds of operations: (direct or indirect) x (callortry_call).According to our need and feasibility of constructing a test case, this PR includes:
call. Until now, the only extra calling conventions are SPIR ones, which cannot be set from source code manually using attributes. Meanwhile, OpenCL C does not allow function pointers or exceptions, therefore the only case remaining is direct call.call, but not anytry_call. Although it's possible to write all 4 kinds of calls with calling convention in ClangIR assembly, exceptions is quite hard to write and read. I prefer source-code-level test for it when it's available in the future. For example, possibly C++thiscallwith exceptions.calland directtry_call.All unsupported cases are guarded by assertions or MLIR diags.