[PATCH] [RFC] New early __builtin_unreachable processing.

Andrew MacLeod amacleod@redhat.com
Fri Sep 15 14:44:55 GMT 2023


Ive been looking at __builtin_unreachable () regressions.  The 
fundamental problem seems to be  a lack of consistent expectation for 
when we remove it earlier than the final pass of VRP.    After looking 
through them, I think this provides a sensible approach.

Ranger is pretty good at providing ranges in blocks dominated by the 
__builtin_unreachable  branch, so removing it isn't quite a critical as 
it once was.  Its also pretty good at identifying what in the block can 
be affected by the branch.

This patch provide an alternate removal algorithm for earlier passes.  
it looks at *all* the exports from the block, and if the branch 
dominates every use of all the exports, AND none of those values access 
memory, VRP will remove the unreachable call, rewrite the branch, update 
all the values globally, and finally perform the simple DCE on the 
branch's ssa-name.   This is kind of what it did before, but it wasn't 
as stringent on the requirements.

The memory access check is required because there are a couple of test 
cases for PRE in which there is a series of instruction leading to an 
unreachable call, and none of those ssa names are ever used in the IL 
again. The whole chunk is dead, and we update globals, however 
pointlessly.  However, one of ssa_names loads from memory, and a later 
passes commons this value with a later load, and then  the unreachable 
call provides additional information about the later load.    This is 
evident in tree-ssa/ssa-pre-34.c.   The only way I see to avoid this 
situation is to not remove the unreachable if there is a load feeding it.

What this does is a more sophisticated version of what DOM does in 
all_uses_feed_or_dominated_by_stmt.  THe feeding instructions dont have 
to be single use, but they do have to be dominated by the branch or be 
single use within the branches block..

If there are multiple uses in the same block as the branch, this does 
not remove the unreachable call.  If we could be sure there are no 
intervening calls or side effects, it would be allowable, but this a 
more expensive checking operation.  Ranger gets the ranges right anyway, 
so with more passes using ranger, Im not sure we'd see much benefit from 
the additional analysis.   It could always be added later.

This fixes at least 110249 and 110080 (and probably others).  The only 
regression is 93917 for which I changed the testcase to adjust 
expectations:

// PR 93917
void f1(int n)
{
   if(n<0)
     __builtin_unreachable();
   f3(n);
}

void f2(int*n)
{
   if(*n<0)
     __builtin_unreachable();
   f3 (*n);
}

We were removing both unreachable calls in VRP1, but only updating the 
global values in the first case, meaning we lose information.   With the 
change in semantics, we only update the global in the first case, but we 
leave the unreachable call in the second case now (due to the load from 
memory).  Ranger still calculates the contextual range correctly as [0, 
+INF] in the second case, it just doesn't set the global value until 
VRP2 when it is removed.

Does this seem reasonable?

Bootstraps on x86_64-pc-linux-gnu with no regressions.  OK?

Andrew


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-New-early-__builtin_unreachable-processing.patch
Type: text/x-patch
Size: 14587 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20230915/4a5013fc/attachment-0001.bin>


More information about the Gcc-patches mailing list