Skip to content

Make cir.call aware of calling conventions #803

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
seven-mile opened this issue Aug 23, 2024 · 1 comment · Fixed by #828
Closed

Make cir.call aware of calling conventions #803

seven-mile opened this issue Aug 23, 2024 · 1 comment · Fixed by #828
Labels
invalid This doesn't seem right

Comments

@seven-mile
Copy link
Collaborator

LLVM has both callee (Function) and callsite (CallInst) attributed by calling conv. Currently CIR always set default calling conv (CConv::C) when lowering to LLVM call / invoke ops, which produces wrong LLVM IR for callsites.


Here are some considerations from my side, FYI.

First of all, I'm not sure whether ClangIR should allow mismatch of call convs between callsite and callee: LLVM allows it, but I think Clang AST does not. In Clang AST, the calling conv is stored in type system (clang::FunctionType::ExtInfo). Thus, I lean towards the latter (they must match).

Then, we have two typical approaches here, again ; )

  • Extend !cir.func<> type with calling convention attribute. Infer the call conv for callsite when lowering.
    • This provides a straightforward equivalent to Clang AST, and defers the conversion until lowering.
  • Extend the cir.call operations directly and keep close with OG skeleton.
    • By utilizing the information from Clang AST, we can still easily determine the calling convention of the callsite in corner cases like invoking a function pointer.

I think both are workable for just correctness. But the first one preserve more information for analysis and cost more (more allocation size for every function type). FWIW: The so-called "more information" is the calling conventions of intermediate results of functions, which is not common. Most of the time, we are just doing bitcast of function pointers. I don't think such info involves vital optimizations.

@seven-mile
Copy link
Collaborator Author

Update: Note that LLVM also does NOT allow mismatch between call convs of call and func. It's undefined behavior and would be optimized into unreachable for some passes.

Hugobros3 pushed a commit to shady-gang/clangir that referenced this issue Oct 2, 2024
)

The first patch to fix llvm#803 . This PR adds the calling convention
attribute to CallOp directly, which is similar to LLVM, rather than
adding the information to function type, which mimics Clang AST function
type.

The syntax of it in CIR assembly is between the function type and extra
attributes, as follows:

```mlir
%1 = cir.call %fnptr(%a) : (!fnptr, !s32i) -> !s32i cc(spir_kernel) extra(#fn_attr)
```

The verification of direct calls is not included. It will be included in
the next patch extending CIRGen & Lowering.

---

For every builder method of Call Op, an optional parameter `callingConv`
is inserted right before the parameter of extra attribute. However,
apart from the parser / printer, this PR does not introduce any
functional changes.
smeenai pushed a commit to smeenai/clangir that referenced this issue Oct 9, 2024
)

The first patch to fix llvm#803 . This PR adds the calling convention
attribute to CallOp directly, which is similar to LLVM, rather than
adding the information to function type, which mimics Clang AST function
type.

The syntax of it in CIR assembly is between the function type and extra
attributes, as follows:

```mlir
%1 = cir.call %fnptr(%a) : (!fnptr, !s32i) -> !s32i cc(spir_kernel) extra(#fn_attr)
```

The verification of direct calls is not included. It will be included in
the next patch extending CIRGen & Lowering.

---

For every builder method of Call Op, an optional parameter `callingConv`
is inserted right before the parameter of extra attribute. However,
apart from the parser / printer, this PR does not introduce any
functional changes.
smeenai pushed a commit to smeenai/clangir that referenced this issue Oct 9, 2024
)

The first patch to fix llvm#803 . This PR adds the calling convention
attribute to CallOp directly, which is similar to LLVM, rather than
adding the information to function type, which mimics Clang AST function
type.

The syntax of it in CIR assembly is between the function type and extra
attributes, as follows:

```mlir
%1 = cir.call %fnptr(%a) : (!fnptr, !s32i) -> !s32i cc(spir_kernel) extra(#fn_attr)
```

The verification of direct calls is not included. It will be included in
the next patch extending CIRGen & Lowering.

---

For every builder method of Call Op, an optional parameter `callingConv`
is inserted right before the parameter of extra attribute. However,
apart from the parser / printer, this PR does not introduce any
functional changes.
keryell pushed a commit to keryell/clangir that referenced this issue Oct 19, 2024
)

The first patch to fix llvm#803 . This PR adds the calling convention
attribute to CallOp directly, which is similar to LLVM, rather than
adding the information to function type, which mimics Clang AST function
type.

The syntax of it in CIR assembly is between the function type and extra
attributes, as follows:

```mlir
%1 = cir.call %fnptr(%a) : (!fnptr, !s32i) -> !s32i cc(spir_kernel) extra(#fn_attr)
```

The verification of direct calls is not included. It will be included in
the next patch extending CIRGen & Lowering.

---

For every builder method of Call Op, an optional parameter `callingConv`
is inserted right before the parameter of extra attribute. However,
apart from the parser / printer, this PR does not introduce any
functional changes.
lanza pushed a commit that referenced this issue Nov 5, 2024
The first patch to fix #803 . This PR adds the calling convention
attribute to CallOp directly, which is similar to LLVM, rather than
adding the information to function type, which mimics Clang AST function
type.

The syntax of it in CIR assembly is between the function type and extra
attributes, as follows:

```mlir
%1 = cir.call %fnptr(%a) : (!fnptr, !s32i) -> !s32i cc(spir_kernel) extra(#fn_attr)
```

The verification of direct calls is not included. It will be included in
the next patch extending CIRGen & Lowering.

---

For every builder method of Call Op, an optional parameter `callingConv`
is inserted right before the parameter of extra attribute. However,
apart from the parser / printer, this PR does not introduce any
functional changes.
lanza pushed a commit that referenced this issue Mar 18, 2025
The first patch to fix #803 . This PR adds the calling convention
attribute to CallOp directly, which is similar to LLVM, rather than
adding the information to function type, which mimics Clang AST function
type.

The syntax of it in CIR assembly is between the function type and extra
attributes, as follows:

```mlir
%1 = cir.call %fnptr(%a) : (!fnptr, !s32i) -> !s32i cc(spir_kernel) extra(#fn_attr)
```

The verification of direct calls is not included. It will be included in
the next patch extending CIRGen & Lowering.

---

For every builder method of Call Op, an optional parameter `callingConv`
is inserted right before the parameter of extra attribute. However,
apart from the parser / printer, this PR does not introduce any
functional changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant