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

[X86] bt regression with bmi #55138

Closed
sftlbcn opened this issue Apr 27, 2022 · 3 comments
Closed

[X86] bt regression with bmi #55138

sftlbcn opened this issue Apr 27, 2022 · 3 comments
Assignees

Comments

@sftlbcn
Copy link

sftlbcn commented Apr 27, 2022

looks worse with bmi?
godbolt: https://godbolt.org/z/aj3so3noj

int test (int x)
{
  x &= 0xf;
  return (0x6996 >> x) & 1;
}

trunk:

test(int): # @test(int)
  and dil, 15
  movzx ecx, dil
  mov edx, 27030
  xor eax, eax
  bt edx, ecx
  setb al
  ret

clang 14:

test(int): # @test(int)
  mov ecx, edi
  and cl, 15
  mov eax, 27030
  shr eax, cl
  and eax, 1
  ret

clang 14 with bmi2:

test(int): # @test(int)
  and dil, 15
  mov eax, 27030
  shrx eax, eax, edi
  and eax, 1
  ret
@phoebewang
Copy link
Contributor

Didn't verify, but I think it's changed by https://reviews.llvm.org/D122891
It is an optimization, since shr with reg is slow.

@phoebewang
Copy link
Contributor

phoebewang commented Apr 27, 2022

My mistake, bmi is better https://godbolt.org/z/rWj8bn8j6

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 27, 2022

@llvm/issue-subscribers-backend-x86

@RKSimon RKSimon self-assigned this Apr 27, 2022
RKSimon added a commit that referenced this issue Apr 27, 2022
RKSimon added a commit that referenced this issue Apr 27, 2022
As discussed on Issue #55138 - BMI2 (fast shrx) shouldn't always fold to BT
RKSimon added a commit that referenced this issue Apr 28, 2022
Ideally we'd fold this with generic DAGCombiner, but that only works for !isTruncateFree cases - we might be able to adapt IsDesirableToPromoteOp to find truncated src ops in the future, but for now just use this peephole.

Noticed in Issue #55138
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

5 participants