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

[InstCombine] Failure to fold sext(trunc(ashr(x,bw-1)) -> sext(ashr(x,bw-1) #48887

Closed
RKSimon opened this issue Mar 11, 2021 · 9 comments
Closed
Assignees
Labels
bugzilla Issues migrated from bugzilla

Comments

@RKSimon
Copy link
Collaborator

RKSimon commented Mar 11, 2021

Bugzilla Link 49543
Resolution FIXED
Resolved on Jun 04, 2021 00:33
Version trunk
OS Windows NT
CC @LebedevRI,@nikic,@rotateright
Fixed by commit(s) b865eea

Extended Description

Alive2: https://alive2.llvm.org/ce/z/A3FMem

define i64 @​src(i32 %a0) {
%a = ashr i32 %a0, 16 ; all valid 16 -> 31
%b = trunc i32 %a to i16
%c = sext i16 %b to i64
ret i64 %c
}

define i64 @​tgt(i32 %a0) {
%a = ashr i32 %a0, 16
%c = sext i32 %a to i64
ret i64 %c
}

https://c.godbolt.org/z/zh7ffM

@RKSimon
Copy link
Collaborator Author

RKSimon commented Mar 11, 2021

assigned to @rotateright

@LebedevRI
Copy link
Member

Same is true for lshr:
https://alive2.llvm.org/ce/z/NFowLr

@LebedevRI
Copy link
Member

Err, submitted too soon

Same is true for lshr:
https://alive2.llvm.org/ce/z/NFowLr
https://c.godbolt.org/z/7q545n

I think we could use ComputeNumSignBits() here.
I'll try to take a look..

Same for zext, but we get that already: https://c.godbolt.org/z/6GqxTM
https://alive2.llvm.org/ce/z/289tfg
https://alive2.llvm.org/ce/z/GWh-uX

@RKSimon
Copy link
Collaborator Author

RKSimon commented Apr 13, 2021

@​lebedev.ri Are you still looking at this?

@LebedevRI
Copy link
Member

@​lebedev.ri Are you still looking at this?

Not really. If someone wants to pick this up, feel free to.
There is a number of patches needed here:

  • general computenumsignbits fold
  • fold for that specific IR, to handle non-splat vectors
  • fold for the case where the first op is lshr
  • ???
  • do we need something similar for zero-extension?

@rotateright
Copy link
Contributor

Looking at the ComputeNumSignBits pattern - tests:
https://reviews.llvm.org/rG337854270023

@rotateright
Copy link
Contributor

@rotateright
Copy link
Contributor

Filed the 'lshr' pattern as bug 50575.
The example pasted in the description is transformed into that (ashr->lshr).

But the bug title and the example in the godbolt link in the description are different and should be fixed with:
https://reviews.llvm.org/rGb865eead7657

So I think it's ok to close this one, but feel free to reopen or file new bugs to track other items.

@RKSimon
Copy link
Collaborator Author

RKSimon commented Jun 4, 2021

Cheers Sanjay!

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

3 participants