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 fold trunc(ctlz(zext(x))) -> add(ctlz(x), C) #49517

Closed
davidbolvansky opened this issue Apr 29, 2021 · 6 comments
Closed

Missing fold trunc(ctlz(zext(x))) -> add(ctlz(x), C) #49517

davidbolvansky opened this issue Apr 29, 2021 · 6 comments
Labels
bugzilla Issues migrated from bugzilla

Comments

@davidbolvansky
Copy link
Collaborator

Bugzilla Link 50173
Resolution FIXED
Resolved on Jun 29, 2021 10:01
Version trunk
OS Linux
CC @RKSimon,@rotateright
Fixed by commit(s) ad0085d

Extended Description

define i16 @​src(i16 %x) {

%z = zext i16 %x to i32
%p = call i32 @​llvm.ctlz.i32(i32 %z, i1 false)
%zz = trunc i32 %p to i16
ret i16 %zz
}

define i16 @​tgt(i16 %x) {

%p = call i16 @​llvm.ctlz.i16(i16 %x, i1 false)
%z = add i16 %p, 16
ret i16 %z
}

declare i32 @​llvm.ctlz.i32(i32, i1)
declare i16 @​llvm.ctlz.i16(i16, i1)


define i16 @​src(i16 %x) {
%0:
%z = zext i16 %x to i32
%p = ctlz i32 %z, 0
%zz = trunc i32 %p to i16
ret i16 %zz
}
=>
define i16 @​tgt(i16 %x) {
%0:
%p = ctlz i16 %x, 0
%z = add i16 %p, 16
ret i16 %z
}
Transformation seems to be correct!

@davidbolvansky
Copy link
Collaborator Author

For known negative x


define i32 @​src(i16 %x) {
%0:
%i = icmp slt i16 %x, 0
assume i1 %i
%z = sext i16 %x to i32
%p = ctlz i32 %z, 0
ret i32 %p
}
=>
define i32 @​tgt(i16 %x) {
%0:
%p = ctlz i16 %x, 0
%z = zext i16 %p to i32
ret i32 %z
}
Transformation seems to be correct!

@llvmbot
Copy link
Collaborator

llvmbot commented Jun 6, 2021

Hi Sir,
Can I take this up. This will be my first ever contribution to LLVM. So, I might need some help along the way of how to proceed.
I will try to provide a patch in a couple of days for the same.

@RKSimon
Copy link
Collaborator

RKSimon commented Jun 6, 2021

Go for it - you should find similar code inside InstCombineCasts.cpp already that you can reference.

Once you have something its best to create an initial patch on Phabricator and people can help you there with its development.

@llvmbot
Copy link
Collaborator

llvmbot commented Jun 7, 2021

Hi Simon, David,
I have sent my initial patch for review: https://reviews.llvm.org/D103788
This is only for positive numbers as per the first comment from David.
I will work on the negative numbers now, meanwhile can you guys review the positive case first.

I have added unit test as well for a few datatypes. Please have a look.

@davidbolvansky
Copy link
Collaborator Author

Please ignore comment #​2, llvm already handles it.

@llvmbot
Copy link
Collaborator

llvmbot commented Jun 7, 2021

ok

@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