> > > > When this function replaces the edge it doesn't seem to update the > > > dominators. > > > > Since It's replacing the middle BB we then end up with an error > > > > > > > > gcc/testsuite/gcc.dg/tree-ssa/minmax-14.c:17:1: error: dominator > > > > of 5 should be 4, not 2 > > > > > > > > during early verify. So instead, I replace the BB but defer its > > > > deletion until cleanup which removes it and updates the dominators. > > > > > > Hmm, for a diamond shouldn't you replace > > > > > > if (EDGE_SUCC (cond_block, 0)->dest == bb) > > > edge_to_remove = EDGE_SUCC (cond_block, 1); > > > else > > > edge_to_remove = EDGE_SUCC (cond_block, 0); > > > > > > with > > > > > > if (EDGE_SUCC (cond_block, 0)->dest == bb) > > > edge_to_remove = EDGE_SUCC (cond_block, 1); > > > else if (EDGE_SUCC (cond_block, 1)->dest == bb) > > > edge_to_remove = EDGE_SUCC (cond_block, 0); > > > > > > thus, the code expects to be left with a fallthru to the PHI block > > > which is expected to have the immediate dominator being cond_block > > > but with a diamond there's a (possibly empty) block inbetween and > > > dominators are wrong. > > > > Agreed, but the (EDGE_SUCC (cond_block, 1)->dest == bb) doesn't seem > > like the Right one since for a diamond there will be a block in > > between the two. Did you perhaps mean EDGE_SUCC (EDGE_SUCC > > (cond_block, 1)->dest, 0)->dest == bb? i.e. that that destination across the > diamond be bb, and then you remove the middle block? > > Hmm, I think my condition was correct - the code tries to remove the edge to > the middle-block and checks the remaining edge falls through to the merge > block. With a true diamond there is no fallthru to the merge block to keep so > we better don't remove any edge? > > > For the minmax diamond we want both edges removed, since all the code > > in the middle BBs are now dead. But this is probably not true in the > > general > sense.
Ah! Sorry I was firing a few cylinders short, I get what you mean now: @@ -425,8 +439,19 @@ replace_phi_edge_with_variable (basic_block cond_block, edge edge_to_remove; if (EDGE_SUCC (cond_block, 0)->dest == bb) edge_to_remove = EDGE_SUCC (cond_block, 1); - else + else if (EDGE_SUCC (cond_block, 1)->dest == bb) edge_to_remove = EDGE_SUCC (cond_block, 0); + else + { + /* If neither edge from the conditional is the final bb + then we must have a diamond block, in which case + the true edge was changed by SET_USE above and we must + mark the other edge as the false edge. */ + gcond *cond = as_a <gcond *> (last_stmt (cond_block)); + gimple_cond_make_false (cond); + return; + } + Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. Ok with this Change? Thanks, Tamar