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

missing instcombine: select (icmp ugt A, N), (shr A, M), 0 -> shr A, M, when N < 2^M #54089

Closed
zygoloid opened this issue Feb 25, 2022 · 3 comments

Comments

@zygoloid
Copy link
Collaborator

Example, seen in the wild in libcap-ng:

void f(int, int);
void g(unsigned int capability) {
 	int idx;

	if (capability > 31) {
		idx = capability>>5;
		capability %= 32;
	} else
		idx = 0;

        f(idx, capability);
}

produces:

define dso_local void @g(i32 noundef %0) local_unnamed_addr #0 {
  %2 = icmp ugt i32 %0, 31
  %3 = lshr i32 %0, 5
  %4 = and i32 %0, 31
  %5 = select i1 %2, i32 %3, i32 0
  tail call void @f(i32 noundef %5, i32 noundef %4) #2
  ret void
}

This could be optimized further to:

define dso_local void @g(i32 noundef %0) local_unnamed_addr #0 {
  %1 = lshr i32 %0, 5
  %2 = and i32 %0, 31
  tail call void @f(i32 noundef %1, i32 noundef %2) #2
  ret void
}

The general pattern here is:

select (icmp ugt A, N), (shr A, M), 0 -> shr A, M

whenever N < 2M.

@rotateright
Copy link
Contributor

We probably want an even more general match using knownbits (ValueTracking) when the compare (select condition) is a disguised bit-test (see llvm::decomposeBitTestICmp()).

For example, we also miss the optimization with an 'and' instruction instead of a shift:
https://alive2.llvm.org/ce/z/ns7yER

define i8 @src(i8 %x) {
  %is_low_val = icmp ult i8 %x, 32
  %high_mask_x = and i8 %x, -32
  %r = select i1 %is_low_val, i8 0, i8 %high_mask_x
  ret i8 %r
} 

define i8 @tgt(i8 %x) {
  %high_mask_x = and i8 %x, -32
  ret i8 %high_mask_x
} 

@nikic
Copy link
Contributor

nikic commented Feb 27, 2022

I'd probably approach this in terms of icmp(X) ? f(X) : C where f(exact_icmp_range) == C.

@alexander-shaposhnikov
Copy link
Collaborator

I've prepared a patch, planning to send it for review ~soon.

alexander-shaposhnikov added a commit that referenced this issue Apr 12, 2022
This diff extends foldSelectInstWithICmp to handle the case icmp(X) ? f(X) : C
when f(X) is guaranteed to be equal to C for all X in the exact range of the inverse predicate.
This addresses the issue #54089.

Differential revision: https://reviews.llvm.org/D123159

Test plan: make check-all
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this issue Oct 7, 2022
This diff extends foldSelectInstWithICmp to handle the case icmp(X) ? f(X) : C
when f(X) is guaranteed to be equal to C for all X in the exact range of the inverse predicate.
This addresses the issue llvm/llvm-project#54089.

Differential revision: https://reviews.llvm.org/D123159

Test plan: make check-all
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants