Skip to content
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

[CIR][CIRGen][LLVMLowering] Add support for checked arithmetic builtins #560

Merged
merged 2 commits into from
May 5, 2024

Conversation

Lancern
Copy link
Member

@Lancern Lancern commented Apr 22, 2024

This patch adds support for checked arithmetic builtins, including:

  • __builtin_add_overflow and __builtin_{s|u}add{|l|ll}_overflow;
  • __builtin_sub_overflow and __builtin_{s|u}sub{|l|ll}_overflow;
  • __builtin_mul_overflow and __builtin_{s|u}mul{|l|ll}_overflow.

This patch adds a new operation cir.checked_arith to represent these builtins. Unlike other CIR operations, this new operation has two result values. One for the possibly truncated result, and the other for a boolean flag that indicates whether the operation has overflowed.

CIRGen and LLVMIR lowering support for the new operation is both included in this PR.

@Lancern Lancern marked this pull request as draft April 23, 2024 02:19
@Lancern Lancern marked this pull request as ready for review April 23, 2024 15:13
@Lancern Lancern changed the title [CIR][CIRGen] Add CIRGen for checked arithmetic builtins [CIR][CIRGen][LLVMLowering] Add support for checked arithmetic builtins Apr 23, 2024
Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Minor nits in the comments.

clang/include/clang/CIR/Dialect/IR/CIROps.td Outdated Show resolved Hide resolved
clang/include/clang/CIR/Dialect/IR/CIROps.td Outdated Show resolved Hide resolved
clang/include/clang/CIR/Dialect/IR/CIROps.td Show resolved Hide resolved
clang/include/clang/CIR/Dialect/IR/CIROps.td Show resolved Hide resolved
clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp Show resolved Hide resolved
@Lancern Lancern force-pushed the builtins-overflow-cirgen branch from b481a25 to 5dc8797 Compare April 27, 2024 14:50
@Lancern
Copy link
Member Author

Lancern commented Apr 27, 2024

Rebased onto the latest main.

@Lancern
Copy link
Member Author

Lancern commented Apr 28, 2024

Rebased onto the latest main.

Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after a minor nit

clang/include/clang/CIR/Dialect/IR/CIROps.td Outdated Show resolved Hide resolved
@lanza lanza force-pushed the main branch 2 times, most recently from c4db6d0 to e197d4e Compare April 29, 2024 20:11
This patch adds CIRGen support for checked arithmetic builtins, including:

  - `__builtin_add_overflow` and `__builtin_{s|u}add{|l|ll}_overflow`;
  - `__builtin_sub_overflow` and `__builtin_{s|u}sub{|l|ll}_overflow`;
  - `__builtin_mul_overflow` and `__builtin_{s|u}mul{|l|ll}_overflow`.

This patch adds a new operation `cir.binop.overflow` to represent these
builtins. Unlike other CIR operations, this new operation has two result values.
One for the possibly truncated result, and the other for a boolean flag that
indicates whether the operation has overflowed.
@Lancern Lancern force-pushed the builtins-overflow-cirgen branch from 39c7765 to 8ef5113 Compare May 2, 2024 15:50
@Lancern
Copy link
Member Author

Lancern commented May 2, 2024

Rebased onto the latest main.

Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bcardosolopes bcardosolopes merged commit f9b1067 into llvm:main May 5, 2024
6 checks passed
@Lancern Lancern deleted the builtins-overflow-cirgen branch May 6, 2024 03:47
bruteforceboy pushed a commit to bruteforceboy/clangir that referenced this pull request Oct 2, 2024
…ns (llvm#560)

This patch adds support for checked arithmetic builtins, including:

  - `__builtin_add_overflow` and `__builtin_{s|u}add{|l|ll}_overflow`;
  - `__builtin_sub_overflow` and `__builtin_{s|u}sub{|l|ll}_overflow`;
  - `__builtin_mul_overflow` and `__builtin_{s|u}mul{|l|ll}_overflow`.

This patch adds a new operation `cir.checked_arith` to represent these
builtins. Unlike other CIR operations, this new operation has two result
values. One for the possibly truncated result, and the other for a
boolean flag that indicates whether the operation has overflowed.

CIRGen and LLVMIR lowering support for the new operation is both
included in this PR.
Hugobros3 pushed a commit to shady-gang/clangir that referenced this pull request Oct 2, 2024
…ns (llvm#560)

This patch adds support for checked arithmetic builtins, including:

  - `__builtin_add_overflow` and `__builtin_{s|u}add{|l|ll}_overflow`;
  - `__builtin_sub_overflow` and `__builtin_{s|u}sub{|l|ll}_overflow`;
  - `__builtin_mul_overflow` and `__builtin_{s|u}mul{|l|ll}_overflow`.

This patch adds a new operation `cir.checked_arith` to represent these
builtins. Unlike other CIR operations, this new operation has two result
values. One for the possibly truncated result, and the other for a
boolean flag that indicates whether the operation has overflowed.

CIRGen and LLVMIR lowering support for the new operation is both
included in this PR.
keryell pushed a commit to keryell/clangir that referenced this pull request Oct 19, 2024
…ns (llvm#560)

This patch adds support for checked arithmetic builtins, including:

  - `__builtin_add_overflow` and `__builtin_{s|u}add{|l|ll}_overflow`;
  - `__builtin_sub_overflow` and `__builtin_{s|u}sub{|l|ll}_overflow`;
  - `__builtin_mul_overflow` and `__builtin_{s|u}mul{|l|ll}_overflow`.

This patch adds a new operation `cir.checked_arith` to represent these
builtins. Unlike other CIR operations, this new operation has two result
values. One for the possibly truncated result, and the other for a
boolean flag that indicates whether the operation has overflowed.

CIRGen and LLVMIR lowering support for the new operation is both
included in this PR.
lanza pushed a commit that referenced this pull request Nov 5, 2024
…ns (#560)

This patch adds support for checked arithmetic builtins, including:

  - `__builtin_add_overflow` and `__builtin_{s|u}add{|l|ll}_overflow`;
  - `__builtin_sub_overflow` and `__builtin_{s|u}sub{|l|ll}_overflow`;
  - `__builtin_mul_overflow` and `__builtin_{s|u}mul{|l|ll}_overflow`.

This patch adds a new operation `cir.checked_arith` to represent these
builtins. Unlike other CIR operations, this new operation has two result
values. One for the possibly truncated result, and the other for a
boolean flag that indicates whether the operation has overflowed.

CIRGen and LLVMIR lowering support for the new operation is both
included in this PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants