Skip to content

[InstCombine] Transform sub(select, select) into select when they have same conditions #106511

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

Closed
vfdff opened this issue Aug 29, 2024 · 7 comments

Comments

@vfdff
Copy link
Contributor

vfdff commented Aug 29, 2024

define i32 @sub_select_select_src(i32 %a) {
  %cmp = icmp eq i32 %a, 0
  %lhs = select i1 %cmp, i32 1, i32 %a
  %rhs = select i1 %cmp, i32 9, i32 %a
  %res = sub nsw i32 %lhs, %rhs
  ret i32 %res
}
  • we expect have the result after instCombine transform
define i32 @sub_select_select_tgt(i32 %a) {
  %cmp = icmp eq i32 %a, 0
  %res = select i1 %cmp, i32 -8, i32 0
  ret i32 %res
}
@vfdff
Copy link
Contributor Author

vfdff commented Aug 29, 2024

It seems a regression of llvm, https://gcc.godbolt.org/z/Wdq8s9saf

works fine with llvm 16.0

@pedroclobo
Copy link
Member

Hi, I would like to work on this.

@nikic
Copy link
Contributor

nikic commented Aug 29, 2024

Can you please share which real-world code this affects?

@vfdff
Copy link
Contributor Author

vfdff commented Aug 31, 2024

It come from the hot function digits_2 of spec2017, 548.exchange2, related code snippet

  recursive subroutine digits_2(row)
  integer, intent(in) :: row
  integer :: i1, i2, i3, i4, i5, i6, i7, i8, i9, l(r), u(r), rnext, col
  logical :: OK
     
     !  call i32 @llvm.umax.i32(i32 %tmp, i32 1)
     where(sudoku1(row, :) /= 0)
        l = sudoku1(row, :)
         u = l
     elsewhere
         l = 1
        u = r
     end where

     rnext = ((row-1)/3+1)*3+1

  do i1 = l(1), u(1)
     if(block(row, 1, i1) <= 0) cycle
      ...

@nikic
Copy link
Contributor

nikic commented Sep 1, 2024

I believe that #65978 is an existing PR trying to address this issue (cc @davemgreen).

@vfdff
Copy link
Contributor Author

vfdff commented Sep 2, 2024

Thank you very much. This does seem to be a duplicate question.

@vfdff vfdff closed this as completed Sep 2, 2024
@vfdff
Copy link
Contributor Author

vfdff commented Sep 23, 2024

This will be fixed with #107314

@EugeneZelenko EugeneZelenko added the duplicate Resolved as duplicate label Sep 23, 2024
@EugeneZelenko EugeneZelenko closed this as not planned Won't fix, can't repro, duplicate, stale Sep 23, 2024
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

4 participants