Unoptimal CFG optimization for simple select case with -O4

[MaxGraey]

function test(x: i32, y: i32, z: i32): i32 {
  return x ? y : z
}

which is equivalent to:

(func $test (param $p0 i32) (param $p1 i32) (param $p2 i32) (result i32)
  local.get $p0
  if $I0 (result i32)
    local.get $p1
  else
    local.get $p2
  end
)

Currently optimize with (only for -O4) to:

(func $test (param $p0 i32) (param $p1 i32) (param $p2 i32) (result i32)
  local.get $p0
  i32.eqz
  if $I0
    local.get $p2
    local.set $p1
  end
  local.get $p1
)

while optimal and expected output and this btw happening for (-O2 and -O3):

(func $test (param $p0 i32) (param $p1 i32) (param $p2 i32) (result i32)
  local.get $p1
  local.get $p2
  local.get $p0
  select
)

As I understand it relate to Relooper or Flatten pass?

[kripken]

This does look unoptimal.

You can investigate issues like this with BINARYEN_PASS_DEBUG=3 in the environment, it will then dump out the IR after each pass. Reading that, it reminded me that Relooper is not run on -O4. The problem is flatten -> coalesce-locals -> vacuum lead to that final pattern. However, I see that -O4 -O fixes it up - so we do have the optimizations for this, but we end up too near the end of the pipeline to do it without another run.

Perhaps -O4 should run normal opts another time, or we could tweak the passes in some other way, I'm not sure what's best.

[MaxGraey]

You can investigate issues like this with BINARYEN_PASS_DEBUG=3

Nice! Good to know!

Perhaps -O4 should run normal opts another time, or we could tweak the passes in some other way, I'm not sure what's best.

Btw for AS we have our own pass pipeline which better fits to our needs:
https://github.com/AssemblyScript/assemblyscript/blob/master/src/module.ts#L1511

It match with official pipeline in some parts especially at the beginning. It would be great if you gave some advice on how to improve pipeline for this case. And since you are familiar with all the subtleties, maybe you see some non-optimal things in general?

[kripken]

Just running another full run of all default opts (without flatten) should be enough.

I don't see anything in that link that looks obviously unoptimal. But maybe it would be worth comparing that to results from the default passes in binaryen to see if there is a difference.

[MaxGraey]

I don't see anything in that link that looks obviously unoptimal

Thanks for checking!

But maybe it would be worth comparing that to results from the default passes in binaryen to see if there is a difference.

In the beginning it was default pipeline + O4. But over time the pipeline iteratively tuned for our needs.

Regarding this issue what's the best solution? Should we include rereloop pass in our pipeline or better fix/improve flatten binaryen pass here?

[MaxGraey]

Currently fixed via late extra simplify-locals pass