> On 8 Jun 2026, at 09:50, Andrew Pinski <[email protected]> wrote:
> 
> On Mon, Jun 8, 2026 at 12:38 AM Kyrylo Tkachov <[email protected]> wrote:
>> 
>> Hi all,
>> 
>> In the sink pass, if-convert a diamond that selects between two loads at
>> data-dependent addresses (PHI <*P, *Q>) into a single reg-offset load behind
>> branchless selects, reusing phi-opt's factoring helpers (now exported) to 
>> common
>> the load and its address.  Guarded so it only fires on non-vectorisable 
>> in-loop
>> load-address recurrences, leaving affine vectorisable loops alone.  This was 
>> needed
>> to avoid regressing the marian benchmark in SPEC2026 that otherwise ended up 
>> using
>> expensive gathers to vectorise.  We can look to relax this condition in the 
>> future
>> for cases where we think it's beneficial.
>> 
>> The load is commoned only when both arms are guaranteed to reduce to pure
>> speculatable scalar (scc_arms_speculatable_p), so the branchless finish 
>> always
>> succeeds and no commoned-but-still-branching diamond is ever left behind.  
>> The
>> recurrence walk is bounded by a new --param=sink-diamond-recurrence-limit
>> (default 256).  factor_out_conditional_load checks the MEM_REF operand-1 
>> types with
>> alias_ptr_types_compatible_p so loads with incompatible TBAA are never 
>> commoned.
>> 
>> With this patch the Snappy hot loop gets if-converted optimally and 
>> performance
>> improves by 25% on my aarch64 machine, making it a bit better than what LLVM 
>> gets
>> today.
>> 
>> It depends on Andrea's phiopt patch [1] that generalizes commoning out
>> operations beyond unary so they need to go in one after the other.
>> 
>> The code goes from:
>>  .L8:                          ; ---type==0 arm
>>          lsr     x3, x1, 2     ;   tag >> 2
>>          add     x1, x0, x3
>>          add     x3, x3, 2
>>          add     x0, x0, x3    ;   ip += (tag>>2)+2
>>          ldrb    w1, [x1, 1]   ;   LOAD #1: tag = ip[(tag>>2)+1]
>>          cmp     x2, x0
>>          bls     .L2
>>  .L5:                          ;   loop head / else arm
>>          ands    x3, x1, 3     ;   type = tag & 3
>>          beq     .L8           ; <-- DATA-DEPENDENT BRANCH (mispredicts)
>>          ldrb    w1, [x0, x3]  ;   LOAD #2: tag = ip[type]
>>          add     w3, w3, 1
>>          add     x0, x0, x3    ;   ip += type+1
>>          cmp     x2, x0
>>          bhi     .L5
>> 
>> to:
>>  .L3:                          ; --- single loop body, no diamond branch
>>          lsr     x4, x1, 2     ;   tag >> 2
>>          ands    x3, x1, 3     ;   type = tag & 3   (Z = type==0)
>>          csinc   x1, x3, x4, ne ;  offset = (type!=0) ? type : (tag>>2)+1
>>          ldrb    w1, [x0, x1]  ;   ONE reg-offset LOAD: tag = ip[offset]
>>          add     x4, x4, 2
>>          csinc   x3, x4, x3, eq ;  advance = (type==0) ? (tag>>2)+2 : type+1
>>          add     x0, x0, x3    ;   ip += advance
>>          cmp     x2, x0
>>          bhi     .L3
>> 
>> crucially avoiding the badly-predicted branch.
>> 
>> Bootstrapped and tested on aarch64-none-linux-gnu.
>> 
>> Signed-off-by: Kyrylo Tkachov <[email protected]>
>> 
>> gcc/
>> 
>>        PR tree-optimization/125557
>>        * params.opt (sink-diamond-recurrence-limit): New param.
> Just some quick review notes:
> params should be documented in doc/params.texi .
> 
>>        * tree-ssa-phiopt.h (factor_out_conditional_load): Declare.
>>        (factor_out_conditional_operation): Declare.
>>        * tree-ssa-phiopt.cc: Include "alias.h".
>>        (remove_factored_arm_defs): New.
>>        (factor_out_conditional_load): New.
>>        (factor_out_conditional_operation): No longer static.
>>        * tree-ssa-sink.cc (scc_recurrence_p, scc_arms_speculatable_p)
>>        (scc_try_ifconvert, sink_common_computations_to_bb): New.
>>        (pass_sink_code::execute): Call it at the early sink.
> 
> I have an already approved patch which handles:
> ```
> if (a) {
>  if (c) t = abs<d>
>  else t = abs<e>
> }
> ```
> which seemingly will break the sink code here. The only reason why it
> was not applied yet is because I am still convincing myself that the
> order of bb processing in phiopt will work.
> That is it handles number of operands to PHI != 2.
> So can you split this into 2 patches. One for the phiopt piece and one
> for the sink code?

Thanks, I’ve sent out this split into two patches separately.
Kyrill

> 
>> 
>> gcc/testsuite/
>> 
>>        PR tree-optimization/125557
>>        * gcc.dg/tree-ssa/scc-diamond-1.c: New test.
>>        * gcc.dg/tree-ssa/scc-diamond-3.c: New test.
>>        * gcc.target/aarch64/scc-diamond-2.c: New test.
>> 
>> [1] https://gcc.gnu.org/pipermail/gcc-patches/2026-June/719612.html


Reply via email to