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

Support !range metadata for by-value function arguments #76628

Closed
AngelicosPhosphoros opened this issue Dec 30, 2023 · 0 comments · Fixed by #83171
Closed

Support !range metadata for by-value function arguments #76628

AngelicosPhosphoros opened this issue Dec 30, 2023 · 0 comments · Fixed by #83171

Comments

@AngelicosPhosphoros
Copy link

It can be useful for cases like Rusts NonZeroInt types or subrange types like in Pascal,

For example, in Rust, when passing non-zero integer function arguments as references, compiler emits !range metadata on loads so optimizer can exploit this to remove unnecessary checks for zero (and generated branches).
However, when arguments are passed by value (which is optimal in sense of stack memory usage), frontend cannot emit range metadata for function arguments so LLVM optimizer fails to remove zero checks despite them being unnecessary.

E. g. this optimizes well

pub fn is_zero(a: &NonZeroU32)->bool {
   a.get() == 0
}

Output:

define noundef zeroext i1 @is_zero(i32 noundef %_a) unnamed_addr #0 !dbg !14 {
  ret i1 false, !dbg !15
}

and this optimizes badly

pub fn is_zero(a: NonZeroU32)->bool {
   a.get() == 0
}

Output:

define noundef zeroext i1 @is_zero(i32 noundef %a) unnamed_addr #0 !dbg !7 {
  %_0 = icmp eq i32 %a, 0, !dbg !12
  ret i1 %_0, !dbg !13
}

Frontend cannot solve this because even if it would put such the arguments into stack and then load them with metadata specified, first SROA or mem2reg would remove this operations so later optimizations wouldn't see that values have limited range.

@AngelicosPhosphoros AngelicosPhosphoros changed the title Support !range metadata for function argument Support !range metadata for by-value function arguments Dec 30, 2023
AngelicosPhosphoros added a commit to AngelicosPhosphoros/rust that referenced this issue Dec 30, 2023
LLVM currently don't support range metadata for function arguments so it fails to optimize non zero integers using their invariant if they are provided using by-value function arguments.

Related to rust-lang#119422
Related to llvm/llvm-project#76628
AngelicosPhosphoros added a commit to AngelicosPhosphoros/rust that referenced this issue Dec 30, 2023
LLVM currently don't support range metadata for function arguments so it fails to optimize non zero integers using their invariant if they are provided using by-value function arguments.

Related to rust-lang#119422
Related to llvm/llvm-project#76628
Related to rust-lang#49572
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 30, 2023
…get_assume_nonzero, r=<try>

Add assume into `NonZeroIntX::get`

LLVM currently don't support range metadata for function arguments so it fails to optimize non zero integers using their invariant if they are provided using by-value function arguments.

Related to rust-lang#119422
Related to llvm/llvm-project#76628
Related to rust-lang#49572
AngelicosPhosphoros added a commit to AngelicosPhosphoros/rust that referenced this issue Jan 2, 2024
LLVM currently don't support range metadata for function arguments so it fails to optimize non zero integers using their invariant if they are provided using by-value function arguments.

Related to rust-lang#119422
Related to llvm/llvm-project#76628
Related to rust-lang#49572
AngelicosPhosphoros added a commit to AngelicosPhosphoros/rust that referenced this issue Jan 6, 2024
LLVM currently don't support range metadata for function arguments so it fails to optimize non zero integers using their invariant if they are provided using by-value function arguments.

Related to rust-lang#119422
Related to llvm/llvm-project#76628
Related to rust-lang#49572
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 12, 2024
…get_assume_nonzero, r=<try>

Add assume into `NonZeroIntX::get`

LLVM currently don't support range metadata for function arguments so it fails to optimize non zero integers using their invariant if they are provided using by-value function arguments.

Related to rust-lang#119422
Related to llvm/llvm-project#76628
Related to rust-lang#49572
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 12, 2024
…get_assume_nonzero, r=scottmcm

Add assume into `NonZeroIntX::get`

LLVM currently don't support range metadata for function arguments so it fails to optimize non zero integers using their invariant if they are provided using by-value function arguments.

Related to rust-lang#119422
Related to llvm/llvm-project#76628
Related to rust-lang#49572
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Jan 13, 2024
…e_nonzero, r=scottmcm

Add assume into `NonZeroIntX::get`

LLVM currently don't support range metadata for function arguments so it fails to optimize non zero integers using their invariant if they are provided using by-value function arguments.

Related to rust-lang/rust#119422
Related to llvm/llvm-project#76628
Related to rust-lang/rust#49572
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
…e_nonzero, r=scottmcm

Add assume into `NonZeroIntX::get`

LLVM currently don't support range metadata for function arguments so it fails to optimize non zero integers using their invariant if they are provided using by-value function arguments.

Related to rust-lang/rust#119422
Related to llvm/llvm-project#76628
Related to rust-lang/rust#49572
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
…e_nonzero, r=scottmcm

Add assume into `NonZeroIntX::get`

LLVM currently don't support range metadata for function arguments so it fails to optimize non zero integers using their invariant if they are provided using by-value function arguments.

Related to rust-lang/rust#119422
Related to llvm/llvm-project#76628
Related to rust-lang/rust#49572
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants