[CIR][Transform] Add ternary simplification #809

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.

Sign up for GitHub

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

Jump to bottom

Merged

bcardosolopes merged 1 commit into llvm:main from Lancern:ternary-fold-to-select

Sep 11, 2024

Member

Lancern commented Aug 28, 2024 •

edited

Loading

This PR adds a new transformation that transform suitable ternary operations into select operations.

Currently the "suitable" ternary operations are those ternary operations whose both branches satisfy either one of the following criteria:

The branch only contain a single cir.yield operation;
The branch contains a cir.const followed by a cir.yield that yields the constant value produced by the cir.const.
~~The branch contains a cir.load followed by a cir.yield that yields the value loaded by the cir.load. The load operation cannot be volatile and must load from an alloca.~~

These criteria are hardcoded now so that simple C/C++ ternary expressions could be eventually lowered to a cir.select operation instead.

Lancern requested review from lanza and bcardosolopes as code owners

August 28, 2024 15:21

Lancern force-pushed the ternary-fold-to-select branch from 92339bf to 9ae5932 Compare

August 28, 2024 16:14

Lancern commented

View reviewed changes

clang/test/CIR/CodeGen/binop.cpp Outdated Show resolved Hide resolved

bcardosolopes requested changes

View reviewed changes

Member

bcardosolopes left a comment

Thanks for adding this, few comments!

clang/test/CIR/Transforms/ternary-fold.cpp Outdated Show resolved Hide resolved

clang/lib/CIR/Dialect/Transforms/CIRSimplify.cpp Outdated Show resolved Hide resolved

Lancern force-pushed the ternary-fold-to-select branch from 9ae5932 to 2f023a4 Compare

August 29, 2024 13:47

bcardosolopes requested changes

View reviewed changes

clang/lib/CIR/Dialect/Transforms/CIRSimplify.cpp Outdated Show resolved Hide resolved

clang/test/CIR/Transforms/ternary-fold.cir Show resolved Hide resolved

clang/test/CIR/Transforms/ternary-fold.cpp Show resolved Hide resolved

clang/lib/CIR/Dialect/Transforms/CIRSimplify.cpp Show resolved Hide resolved

clang/lib/CIR/Dialect/Transforms/CIRSimplify.cpp Outdated Show resolved Hide resolved

Lancern force-pushed the ternary-fold-to-select branch from 2f023a4 to d9c5e68 Compare

August 31, 2024 03:05

Lancern mentioned this pull request

[CIR][Transform] Add simplify transformation for select op #816

Merged

bcardosolopes requested changes

View reviewed changes

Member

bcardosolopes left a comment

Sorry about the delay, just got back from vacation. This looks mostly good, one minor comment + there are some failing tests.

clang/lib/CIR/Dialect/Transforms/CIRSimplify.cpp Show resolved Hide resolved

bcardosolopes pushed a commit that referenced this pull request


          [CIR][Transform] Add simplify transformation for select op (#816)

8b2274d

As mentioned at
#809 (comment) , this
PR adds more simplify transformations for select op:

- `cir.select if %0 then x else x` -> `x`
- `cir.select if %0 then #true else #false` -> `%0`
- `cir.select if %0 then #false else #true` -> `cir.unary not %0`

Lancern force-pushed the ternary-fold-to-select branch from d9c5e68 to 7f33374 Compare

September 10, 2024 13:30

Member Author

Lancern commented Sep 10, 2024

Rebased onto the latest main.

bcardosolopes approved these changes

View reviewed changes

Member

bcardosolopes left a comment

LGTM

clang/lib/CIR/Dialect/Transforms/CIRSimplify.cpp Show resolved Hide resolved

Member

bcardosolopes commented Sep 10, 2024

There are still bot failures, but should be good to go once they are solved


          [CIR][Transform] Add ternary simplification

e2380e3

This patch adds a new transformation that transform suitable ternary operations
into select operations.

Lancern force-pushed the ternary-fold-to-select branch from 7f33374 to e2380e3 Compare

September 11, 2024 16:00

Lancern commented

View reviewed changes

Member Author

Lancern left a comment

Test failures fixed.

clang/lib/CIR/Dialect/Transforms/CIRSimplify.cpp Show resolved Hide resolved

bcardosolopes merged commit fcc2590 into llvm:main

6 checks passed

Lancern deleted the ternary-fold-to-select branch

September 12, 2024 00:44

Lancern mentioned this pull request

[CIR][Lowering] Erase op through rewriter instead of directly #853

Merged

Hugobros3 pushed a commit to shady-gang/clangir that referenced this pull request


          [CIR][Transform] Add simplify transformation for select op (llvm#816)

354191d

As mentioned at
llvm#809 (comment) , this
PR adds more simplify transformations for select op:

- `cir.select if %0 then x else x` -> `x`
- `cir.select if %0 then #true else #false` -> `%0`
- `cir.select if %0 then #false else #true` -> `cir.unary not %0`

Hugobros3 pushed a commit to shady-gang/clangir that referenced this pull request


          [CIR][Transform] Add ternary simplification (llvm#809)

738fe0d

This PR adds a new transformation that transform suitable ternary
operations into select operations.

Currently the "suitable" ternary operations are those ternary operations
whose both branches satisfy either one of the following criteria:

- The branch only contain a single `cir.yield` operation;
- The branch contains a `cir.const` followed by a `cir.yield` that
yields the constant value produced by the `cir.const`.
- ~~The branch contains a `cir.load` followed by a `cir.yield` that
yields the value loaded by the `cir.load`. The load operation cannot be
volatile and must load from an alloca.~~

These criteria are hardcoded now so that simple C/C++ ternary
expressions could be eventually lowered to a `cir.select` operation
instead.

smeenai pushed a commit to smeenai/clangir that referenced this pull request


          [CIR][Transform] Add simplify transformation for select op (llvm#816)

b56eb7a

As mentioned at
llvm#809 (comment) , this
PR adds more simplify transformations for select op:

- `cir.select if %0 then x else x` -> `x`
- `cir.select if %0 then #true else #false` -> `%0`
- `cir.select if %0 then #false else #true` -> `cir.unary not %0`

smeenai pushed a commit to smeenai/clangir that referenced this pull request


          [CIR][Transform] Add ternary simplification (llvm#809)

0157baf

This PR adds a new transformation that transform suitable ternary
operations into select operations.

Currently the "suitable" ternary operations are those ternary operations
whose both branches satisfy either one of the following criteria:

- The branch only contain a single `cir.yield` operation;
- The branch contains a `cir.const` followed by a `cir.yield` that
yields the constant value produced by the `cir.const`.
- ~~The branch contains a `cir.load` followed by a `cir.yield` that
yields the value loaded by the `cir.load`. The load operation cannot be
volatile and must load from an alloca.~~

These criteria are hardcoded now so that simple C/C++ ternary
expressions could be eventually lowered to a `cir.select` operation
instead.

smeenai pushed a commit to smeenai/clangir that referenced this pull request


          [CIR][Transform] Add simplify transformation for select op (llvm#816)

61bd24b

As mentioned at
llvm#809 (comment) , this
PR adds more simplify transformations for select op:

- `cir.select if %0 then x else x` -> `x`
- `cir.select if %0 then #true else #false` -> `%0`
- `cir.select if %0 then #false else #true` -> `cir.unary not %0`

smeenai pushed a commit to smeenai/clangir that referenced this pull request


          [CIR][Transform] Add ternary simplification (llvm#809)

9a09f80

This PR adds a new transformation that transform suitable ternary
operations into select operations.

Currently the "suitable" ternary operations are those ternary operations
whose both branches satisfy either one of the following criteria:

- The branch only contain a single `cir.yield` operation;
- The branch contains a `cir.const` followed by a `cir.yield` that
yields the constant value produced by the `cir.const`.
- ~~The branch contains a `cir.load` followed by a `cir.yield` that
yields the value loaded by the `cir.load`. The load operation cannot be
volatile and must load from an alloca.~~

These criteria are hardcoded now so that simple C/C++ ternary
expressions could be eventually lowered to a `cir.select` operation
instead.

keryell pushed a commit to keryell/clangir that referenced this pull request


          [CIR][Transform] Add simplify transformation for select op (llvm#816)

410d638

As mentioned at
llvm#809 (comment) , this
PR adds more simplify transformations for select op:

- `cir.select if %0 then x else x` -> `x`
- `cir.select if %0 then #true else #false` -> `%0`
- `cir.select if %0 then #false else #true` -> `cir.unary not %0`

keryell pushed a commit to keryell/clangir that referenced this pull request


          [CIR][Transform] Add ternary simplification (llvm#809)

c1e80e1

This PR adds a new transformation that transform suitable ternary
operations into select operations.

Currently the "suitable" ternary operations are those ternary operations
whose both branches satisfy either one of the following criteria:

- The branch only contain a single `cir.yield` operation;
- The branch contains a `cir.const` followed by a `cir.yield` that
yields the constant value produced by the `cir.const`.
- ~~The branch contains a `cir.load` followed by a `cir.yield` that
yields the value loaded by the `cir.load`. The load operation cannot be
volatile and must load from an alloca.~~

These criteria are hardcoded now so that simple C/C++ ternary
expressions could be eventually lowered to a `cir.select` operation
instead.

lanza pushed a commit that referenced this pull request


          [CIR][Transform] Add simplify transformation for select op (#816)

cb38c58

As mentioned at
#809 (comment) , this
PR adds more simplify transformations for select op:

- `cir.select if %0 then x else x` -> `x`
- `cir.select if %0 then #true else #false` -> `%0`
- `cir.select if %0 then #false else #true` -> `cir.unary not %0`

lanza pushed a commit that referenced this pull request


          [CIR][Transform] Add ternary simplification (#809)

d1800d5

This PR adds a new transformation that transform suitable ternary
operations into select operations.

Currently the "suitable" ternary operations are those ternary operations
whose both branches satisfy either one of the following criteria:

- The branch only contain a single `cir.yield` operation;
- The branch contains a `cir.const` followed by a `cir.yield` that
yields the constant value produced by the `cir.const`.
- ~~The branch contains a `cir.load` followed by a `cir.yield` that
yields the value loaded by the `cir.load`. The load operation cannot be
volatile and must load from an alloca.~~

These criteria are hardcoded now so that simple C/C++ ternary
expressions could be eventually lowered to a `cir.select` operation
instead.

lanza pushed a commit that referenced this pull request


          [CIR][Transform] Add simplify transformation for select op (#816)

07da011

As mentioned at
#809 (comment) , this
PR adds more simplify transformations for select op:

- `cir.select if %0 then x else x` -> `x`
- `cir.select if %0 then #true else #false` -> `%0`
- `cir.select if %0 then #false else #true` -> `cir.unary not %0`

lanza pushed a commit that referenced this pull request


          [CIR][Transform] Add ternary simplification (#809)

16ce22c

This PR adds a new transformation that transform suitable ternary
operations into select operations.

Currently the "suitable" ternary operations are those ternary operations
whose both branches satisfy either one of the following criteria:

- The branch only contain a single `cir.yield` operation;
- The branch contains a `cir.const` followed by a `cir.yield` that
yields the constant value produced by the `cir.const`.
- ~~The branch contains a `cir.load` followed by a `cir.yield` that
yields the value loaded by the `cir.load`. The load operation cannot be
volatile and must load from an alloca.~~

These criteria are hardcoded now so that simple C/C++ ternary
expressions could be eventually lowered to a `cir.select` operation
instead.

xlauko pushed a commit to trailofbits/instafix-llvm that referenced this pull request


          [CIR][Transform] Add simplify transformation for select op (llvm#816)

57758ca

As mentioned at
llvm/clangir#809 (comment) , this
PR adds more simplify transformations for select op:

- `cir.select if %0 then x else x` -> `x`
- `cir.select if %0 then #true else #false` -> `%0`
- `cir.select if %0 then #false else #true` -> `cir.unary not %0`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet