On Mon, Jun 8, 2026 at 3:19 AM Kyrylo Tkachov <[email protected]> wrote: > > Hi all, > > Export phi-opt's factoring helpers so the sink pass (in the following patch) > can reuse them to if-convert data-dependent load diamonds. > factor_out_conditional_operation is made non-static, and a new > factor_out_conditional_load turns a diamond PHI <*P, *Q> that selects between > two loads at data-dependent addresses into a single selected-pointer load. > 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. > > Bootstrapped and tested on aarch64-none-linux-gnu. > Ok for trunk?
The biggest worry I have is: + || !alias_ptr_types_compatible_p (TREE_TYPE (TREE_OPERAND (ref0, 1)), + TREE_TYPE (TREE_OPERAND (ref1, 1)))) Or do something similar to get_alias_type_for_stmts in gimple-ssa-store-merging.cc which merges the sets; well it keeps the original alias type if possible (and the original clique/base) and if the aliasing types are not compatible, uses ptr_type_node (which is the alias all type). I think we should do that here too rather than reject the different aliasing sets. ... + tree newres = make_ssa_name (TREE_TYPE (res)); + /* Reuse REF0's (validated-equal) operand 1; it is a zero INTEGER_CST, hence + shareable, so no unshare_expr is needed. */ + tree nref = build2 (MEM_REF, TREE_TYPE (ref0), ptr, TREE_OPERAND (ref0, 1)); + gassign *load = gimple_build_assign (newres, nref); + gimple_set_vuse (load, gimple_vuse (d0)); + gimple_stmt_iterator gsi = gsi_after_labels (merge); + gsi_insert_before (&gsi, load, GSI_SAME_STMT); + + replace_uses_by (res, newres); Why not reuse the original `gimple_phi_result (phi);` here? Instead of building a new ssa_name that you are just going to replace the old one with the new one? So then you can just do: ``` gsi = gsi_for_stmt (phi); gsi_remove (&gsi, true); ``` instead of using remove_phi_node. This is how factor_out_conditional_operation works and you keep around the original range info if there is any (e.g. from unreachable blocks). Put the tree-ssa-phiopt.h changes/removing of the static in patch 2 instead of this one. Otherwise this looks good once I push my factor_out_conditional_operation changes. > > Thanks, > Kyrill > > Signed-off-by: Kyrylo Tkachov <[email protected]> > > PR tree-optimization/125557 > * 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): Remove static. >
