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

test + multiple cmov instructions causes duplicate conditional variables #2901

Closed
duk-37 opened this issue Jan 30, 2022 · 2 comments
Closed
Labels
State: Duplicate Issue is a duplicate of another issue Type: Enhancement Issue is a small enhancement to existing functionality

Comments

@duk-37
Copy link

duk-37 commented Jan 30, 2022

Version and Platform (required):

  • Binary Ninja Version: 3.0.3238-dev
  • OS: Windows 11
  • OS Version: 22000.376

Bug Description:
A new conditional variable is created every time the result of a test instruction is used.

Steps To Reproduce:
Open the following assembly in any IL view:

test rdx, rdx

cmovne rdx, rcx
cmovne rdx, rcx
cmovne rdx, rcx
cmovne rdx, rcx
cmovne rdx, rcx

mov rax, rdx
ret

Expected Behavior:
Only one conditional variable is created.

Screenshots:
image
image
image

@rssor rssor added the Type: Enhancement Issue is a small enhancement to existing functionality label Jan 30, 2022
@rssor
Copy link
Member

rssor commented Jan 30, 2022

Duplicate of #1720.

This is a missed optimization opportunity but is presently working as intended. When actual inlining of the condition into the use can’t occur (presently this is limited to immediately adjacent instructions) we spill to a temp cond for each use site for a variety of reasons relating to when the decision to spill has to be made and what is known about other uses at the time. It’s too conservative and could get away with only emitting the condition once but it’s been on the backburner since there’s no correctness impact making it a cosmetic issue.

@rssor rssor closed this as completed Jan 30, 2022
@duk-37
Copy link
Author

duk-37 commented Jan 30, 2022

Ah, missed that other issue, sorry.

@psifertex psifertex added the State: Duplicate Issue is a duplicate of another issue label May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
State: Duplicate Issue is a duplicate of another issue Type: Enhancement Issue is a small enhancement to existing functionality
Projects
None yet
Development

No branches or pull requests

3 participants