Skip to content

inlining: optimize and_int(x, true) and or_int(x, false) #54731

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aviatesk
Copy link
Member

@aviatesk aviatesk commented Jun 7, 2024

We need an additional inlining pass to optimize cases like and_int(x, true) and or_int(x, false) to x.

@aviatesk aviatesk force-pushed the avi/opt-and-or-int branch from 00ef18e to 6372248 Compare June 7, 2024 17:11
@vtjnash
Copy link
Member

vtjnash commented Jun 7, 2024

It is not entirely true, since it actually is equivalent to the type-assert call x::Bool. Is there a reason we need this in Julia, rather than letting it occur in LLVM after the IR simplifications?

@vtjnash vtjnash added compiler:optimizer Optimization passes (mostly in base/compiler/ssair/) feature Indicates new feature / enhancement requests labels Jun 7, 2024
@aviatesk aviatesk force-pushed the avi/opt-and-or-int branch from 6372248 to 4494e75 Compare June 8, 2024 04:32
@aviatesk
Copy link
Member Author

aviatesk commented Jun 8, 2024

LLVM does not seem to optimize these intrinsic patterns:

julia> code_llvm((Bool,)) do x
           x & true
       end
; Function Signature: var"#10"(Bool)
;  @ REPL[1]:2 within `#10`
define i8 @"julia_#10_2272"(i8 zeroext %"x::Bool") #0 {
top:
; ┌ @ bool.jl:38 within `&`
   %0 = and i8 %"x::Bool", 1
   ret i8 %0
; └
}

And I think simplifications for such low-level intrinsics should be done early on?

@aviatesk aviatesk force-pushed the avi/opt-and-or-int branch from 4494e75 to 574261f Compare June 8, 2024 04:34
@vchuravy
Copy link
Member

vchuravy commented Jun 8, 2024

That's because LLVM doesn't know that the upper seven bits are ignored...

We used to represent bool as i1, but that caused other problems #17225

@aviatesk aviatesk force-pushed the avi/opt-and-or-int branch from 574261f to e306bfb Compare June 11, 2024 13:38
@vchuravy
Copy link
Member

Clang btw still uses i1 for Bool, so it might be worthwhile trying to switch the implementation back.

@oscardssmith
Copy link
Member

I think our medium term goal is to switch it back. We just have to first make sure that bools on construction always have valid bit-patterns. I believe the plan for how to do this is to always zero initialize bytes that have store bools.

We need an additional inlining pass to optimize cases like
`and_int(x, true)` and `or_int(x, false)` to `x`.
@aviatesk aviatesk force-pushed the avi/opt-and-or-int branch from e306bfb to 1bb8240 Compare June 13, 2024 16:40
@giordano
Copy link
Member

Relevant issue about switching to i1: #21712

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:optimizer Optimization passes (mostly in base/compiler/ssair/) feature Indicates new feature / enhancement requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants