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.

Sign up for GitHub

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

Jump to bottom

[SimplifyCFG] Simplify switch instruction that has duplicate arms #114262

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[SimplifyCFG] Simplify switch instruction that has duplicate arms #114262

wants to merge 4 commits into from
+254 −12

Commits on Oct 30, 2024

  1. [SimplifyCFG] precommit tests for simplify switch with duplicate arms

    @michaelmaitland
    michaelmaitland committed Oct 30, 2024
    Configuration menu
    Copy the full SHA
    fbfa46a View commit details
    Browse the repository at this point in the history

  2. [SimplifyCFG] Simplify switch instruction that has duplicate arms 

    I noticed that the two C functions emitted different IR:

```
int switch_duplicate_arms(int switch_val, int v, int w) {
  switch (switch_val) {
  default:
    break;
  case 0:
    w = v;
    break;
  case 1:
    w = v;
    break;
  }
  return w;
}

int if_duplicate_arms(int switch_val, int v, int w) {
  if (switch_val == 0)
    w = v;
  else if (switch_val == 1)
    w = v;
  return v0;
}
```

For `switch_duplicate_arms`, we generate IR that looks like this:

```
define i32 @switch_duplicate_arms(i32 %0, i32 %1, i32 %2, i32 %3) {
  switch i32 %1, label %7 [
    i32 0, label %5
    i32 1, label %6
  ]

5:
  br label %7

6:
  br label %7

7:
  %8 = phi i32 [ %3, %4 ], [ %2, %6 ], [ %2, %5 ]
  ret i32 %8
}
```

For the equivalent `if_duplicate_arms`, we generate:
```
define i32 @if_duplicate_arms(i32 %0, i32 %1, i32 %2, i32 %3) {
  %5 = icmp ult i32 %1, 2
  %6 = select i1 %5, i32 %2, i32 %3
  ret i32 %6
}
```

For `switch_duplicate_arms`, taking case 0 and 1 are the same since %5 and %6
branch to the same location and the incoming values for %8 are the same from
those blocks. We could remove one on the duplicate switch targets and update
the switch with the single target.

On RISC-V, prior to this patch, we generate the following code:
```
switch_duplicate_arms:
        li      a4, 1
        beq     a1, a4, .LBB0_2
        mv      a0, a3
        bnez    a1, .LBB0_3
.LBB0_2:
        mv      a0, a2
.LBB0_3:
        ret

if_duplicate_arms:
        li      a4, 2
        mv      a0, a2
        bltu    a1, a4, .LBB1_2
        mv      a0, a3
.LBB1_2:
        ret
```

After this patch, the O3 code is optimized to the icmp + select pair, which
gives us the same code gen as `if_duplicate_arms` as desired.

This may help with both code size and further switch simplification. I found
that this patch causes no significant impact to spec2006/int/ref and
spec2017/intrate/ref.
    @michaelmaitland
    michaelmaitland committed Oct 30, 2024
    Configuration menu
    Copy the full SHA
    2db9f00 View commit details
    Browse the repository at this point in the history

  3. fixup! respond to review

    @michaelmaitland
    michaelmaitland committed Oct 30, 2024
    Configuration menu
    Copy the full SHA
    4e56067 View commit details
    Browse the repository at this point in the history

  4. fixup! refactor for general approach

    @michaelmaitland
    michaelmaitland committed Oct 30, 2024
    Configuration menu
    Copy the full SHA
    9e10655 View commit details
    Browse the repository at this point in the history