[CIR][CIRGen] add CIRGen support for assume builtins #841
Conversation
bcardosolopes
left a comment
bcardosolopes
left a comment
There was a problem hiding this comment.
This is neat, thanks! One minor observation
| //===----------------------------------------------------------------------===// | ||
|
|
||
| def AssumeOp : CIR_Op<"assume"> { | ||
| let summary = "Tell the optimizer that a boolean value is true"; |
There was a problem hiding this comment.
We need to mark this operation with some type of side-effect otherwise we'll make the situation with canonicalization worse (it will silently strip those away). We should probably have a simple CIR to CIR test using the canonicalizer and checking they don't get stripped away.
Looks like both AssumeOp and AssumeSepStorageOp need that (but perhaps not AssumeAlignedOp given the result is usually used?)
There was a problem hiding this comment.
Added a test in clang/test/CIR/Transforms/builtin-assume.cir but it seems that neither -cir-simplify nor --canonicalize erases these operations. Are there any other canonicalization passes that could do such optimization?
There was a problem hiding this comment.
That's interesting, I'd have expected these to be doing it (as they usually do for other operations), well at least there's a test to prevent that in case something off is going on in the meantime. Should be good for now, thanks for adding these.
This patch adds CIRGen support for the following 3 builtins related to compile- time assumptions: - `__builtin_assume` - `__builtin_assume_aligned` - `__builtin_assume_separate_storage` 3 new operations are invented to represent the three builtins. LLVMIR lowering for these builtins cannot be implemented at this moment due to the lack of operand bundle support in LLVMIR dialect.
Lancern
force-pushed
the
builtin-assume
branch
from
c6f4616 to
ea067a1
Compare
BTW, I opened a PR in upstream to try to resolve this issue, see llvm/llvm-project#108933 . |
| //===----------------------------------------------------------------------===// | ||
|
|
||
| def AssumeOp : CIR_Op<"assume"> { | ||
| let summary = "Tell the optimizer that a boolean value is true"; |
There was a problem hiding this comment.
That's interesting, I'd have expected these to be doing it (as they usually do for other operations), well at least there's a test to prevent that in case something off is going on in the meantime. Should be good for now, thanks for adding these.
This PR adds CIRGen support for the following 3 builtins related to compile- time assumptions: - `__builtin_assume` - `__builtin_assume_aligned` - `__builtin_assume_separate_storage` 3 new operations are invented to represent the three builtins. _LLVMIR lowering for these builtins cannot be implemented at this moment_ due to the lack of operand bundle support in LLVMIR dialect.
This PR adds CIRGen support for the following 3 builtins related to compile- time assumptions: - `__builtin_assume` - `__builtin_assume_aligned` - `__builtin_assume_separate_storage` 3 new operations are invented to represent the three builtins. _LLVMIR lowering for these builtins cannot be implemented at this moment_ due to the lack of operand bundle support in LLVMIR dialect.
This PR adds CIRGen support for the following 3 builtins related to compile- time assumptions: - `__builtin_assume` - `__builtin_assume_aligned` - `__builtin_assume_separate_storage` 3 new operations are invented to represent the three builtins. _LLVMIR lowering for these builtins cannot be implemented at this moment_ due to the lack of operand bundle support in LLVMIR dialect.
This PR adds CIRGen support for the following 3 builtins related to compile- time assumptions: - `__builtin_assume` - `__builtin_assume_aligned` - `__builtin_assume_separate_storage` 3 new operations are invented to represent the three builtins. _LLVMIR lowering for these builtins cannot be implemented at this moment_ due to the lack of operand bundle support in LLVMIR dialect.
This PR adds CIRGen support for the following 3 builtins related to compile- time assumptions: - `__builtin_assume` - `__builtin_assume_aligned` - `__builtin_assume_separate_storage` 3 new operations are invented to represent the three builtins. _LLVMIR lowering for these builtins cannot be implemented at this moment_ due to the lack of operand bundle support in LLVMIR dialect.
This PR adds CIRGen support for the following 3 builtins related to compile- time assumptions: - `__builtin_assume` - `__builtin_assume_aligned` - `__builtin_assume_separate_storage` 3 new operations are invented to represent the three builtins. _LLVMIR lowering for these builtins cannot be implemented at this moment_ due to the lack of operand bundle support in LLVMIR dialect.
This PR adds CIRGen support for the following 3 builtins related to compile- time assumptions: - `__builtin_assume` - `__builtin_assume_aligned` - `__builtin_assume_separate_storage` 3 new operations are invented to represent the three builtins. _LLVMIR lowering for these builtins cannot be implemented at this moment_ due to the lack of operand bundle support in LLVMIR dialect.
This PR adds CIRGen support for the following 3 builtins related to compile- time assumptions: - `__builtin_assume` - `__builtin_assume_aligned` - `__builtin_assume_separate_storage` 3 new operations are invented to represent the three builtins. _LLVMIR lowering for these builtins cannot be implemented at this moment_ due to the lack of operand bundle support in LLVMIR dialect.
2 participants
This PR adds CIRGen support for the following 3 builtins related to compile- time assumptions:
__builtin_assume__builtin_assume_aligned__builtin_assume_separate_storage3 new operations are invented to represent the three builtins.
LLVMIR lowering for these builtins cannot be implemented at this moment due to the lack of operand bundle support in LLVMIR dialect.