Skip to content

[SimplifyCFG] Switch not fully optimized #63876

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
nikic opened this issue Jul 14, 2023 · 3 comments
Closed

[SimplifyCFG] Switch not fully optimized #63876

nikic opened this issue Jul 14, 2023 · 3 comments

Comments

@nikic
Copy link
Contributor

nikic commented Jul 14, 2023

https://alive2.llvm.org/ce/z/vAWi88

define i8 @src(i8 noundef %arg) {
start:
  switch i8 %arg, label %unreachable [
    i8 0, label %case012
    i8 1, label %case1
    i8 2, label %case2
    i8 3, label %end
  ]

unreachable:
  unreachable

case1:
  br label %case012

case2:
  br label %case012

case012:
  %phi1 = phi i8 [ 3, %case2 ], [ 2, %case1 ], [ 1, %start ]
  br label %end

end:
  %phi2 = phi i8 [ %phi1, %case012 ], [ 4, %start ]
  ret i8 %phi2
}

We should combine this into a single phi, resulting in something like this:

define i8 @tgt(i8 noundef %arg) {
start:
  switch i8 %arg, label %unreachable [
    i8 0, label %end
    i8 1, label %case1
    i8 2, label %case2
    i8 3, label %case3
  ]

unreachable:
  unreachable

case1:
  br label %end

case2:
  br label %end

case3:
  br label %end

end:
  %phi = phi i8 [ 4, %case3 ], [ 3, %case2 ], [ 2, %case1 ], [ 1, %start ]
  ret i8 %phi
}

At which point it is subject to lookup table optimization and converted into "add 1".

@XChy
Copy link
Member

XChy commented Jul 21, 2023

A possible Solution here.

@XChy
Copy link
Member

XChy commented Jul 23, 2023

I agree with your idea of merging phis. However, here I found that lookup optimization doesn't fold the latter IR for the reason that "i8 is not type legal for lookup table" on my x86_64 machine. Actually, it's unfoldable for i32, i64, too. But I think such "LinearMapKind" lookup table should be machine-independent, which may be a bug? What's your opinion here.

XChy added a commit that referenced this issue Sep 25, 2023
…nd SuccBB (#67275)

This patch extends function TryToSimplifyUncondBranchFromEmptyBlock to
handle the similar cases below.

```llvm
define i8 @src(i8 noundef %arg) {
start:
  switch i8 %arg, label %unreachable [
    i8 0, label %case012
    i8 1, label %case1
    i8 2, label %case2
    i8 3, label %end
  ]

unreachable:
  unreachable

case1:
  br label %case012

case2:
  br label %case012

case012:
  %phi1 = phi i8 [ 3, %case2 ], [ 2, %case1 ], [ 1, %start ]
  br label %end

end:
  %phi2 = phi i8 [ %phi1, %case012 ], [ 4, %start ]
  ret i8 %phi2
}
```
The phis here should be merged into one phi, so that we can better
optimize it:

```llvm
define i8 @tgt(i8 noundef %arg) {
start:
  switch i8 %arg, label %unreachable [
    i8 0, label %end
    i8 1, label %case1
    i8 2, label %case2
    i8 3, label %case3
  ]

unreachable:
  unreachable

case1:
  br label %end

case2:
  br label %end

case3:
  br label %end

end:
  %phi = phi i8 [ 4, %case3 ], [ 3, %case2 ], [ 2, %case1 ], [ 1, %start ]
  ret i8 %phi
}
```
Proof:
[normal](https://alive2.llvm.org/ce/z/vAWi88)
[multiple stages](https://alive2.llvm.org/ce/z/DDBQqp)
[multiple stages 2](https://alive2.llvm.org/ce/z/nGkeqN)
[multiple phi combinations](https://alive2.llvm.org/ce/z/VQeEdp)

And lookup table optimization should convert it into add %arg 1.
This patch just match similar CFG structure and merge the phis in
different cases.

Maybe such transform can be applied to other situations besides switch,
but I'm not sure whether it's better than not merging. Therefore, I only
try it in switch,

Related issue:
#63876

[Migrated](https://reviews.llvm.org/D155940)
@XChy
Copy link
Member

XChy commented Sep 25, 2023

#67275 resolves it.

@XChy XChy closed this as completed Sep 25, 2023
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