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] Missed optimization : fold binop(select cond, C1, C2, zext(cond)) #86176

Open
XChy opened this issue Mar 21, 2024 · 4 comments
Open

Comments

@XChy
Copy link
Member

XChy commented Mar 21, 2024

Alive2 proof: https://alive2.llvm.org/ce/z/-f5J6T

Motivating example

define i32 @src(i1 %c) {
entry:
  %cond_zext = zext i1 %c to i32
  %s = select i1 %c, i32 67108863, i32 0
  %or = or i32 %s, %cond_zext
  ret i32 %or
}

can be folded to:

define i32 @tgt(i1 %c) {
entry:
  %s = select i1 %c, i32 67108863, i32 0
  ret i32 %s
}

Real-world motivation

This snippet of IR is derived from qemu/system/physmem.c@phys_page_set_level (after O3 pipeline).
The example above is a reduced version. If you're interested in the original suboptimal IR and optimal IR, please email me.

Let me know if you can confirm that it's an optimization opportunity, thanks.

@XChy
Copy link
Member Author

XChy commented Mar 22, 2024

Generalized proof: https://alive2.llvm.org/ce/z/isMxFR

@gwastar
Copy link

gwastar commented Mar 22, 2024

I started working on this. Should have a patch soon.

@gwastar
Copy link

gwastar commented Mar 23, 2024

This fold already happens for add, sub, and mul and was introduced in commit f12a556. It is performed in foldBinOpOfSelectAndCastOfSelectCondition. This function seems to have two issues: It doesn't preserve flags like nuw, nsw, or disjoint and it allows multi-use selects which I don't think is profitable (see https://godbolt.org/z/vo8GY6xsa and https://alive2.llvm.org/ce/z/AtLvvW). I guess I should fix that function first so that I can use it for this.

@XChy
Copy link
Member Author

XChy commented Mar 23, 2024

@gwastar, yes, I looked into this function. One-use constraint is needed for the select at least.
You could first file a patch to add some constraints on it, and then file the second patch for this issue.

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

2 participants