On 07/10/2026 10:33, Tamar Christina wrote:
-----Original Message----- From: Alfie Richards <[email protected]> Sent: 17 September 2026 15:40 To: [email protected] Cc: Alex Coplan <[email protected]>; Alice Carlotti <[email protected]>; Andrea Pinski <[email protected]>; Richard Biener <[email protected]>; Tamar Christina <[email protected]>; Zdenek Dvorak <[email protected]>; Robin Dapp <[email protected]>; Richard Sandiford <[email protected]>; Kyrylo Tkachov <[email protected]>; Alfie Richards <[email protected]> Subject: [PATCH v3 07/16] vect: Add HSSR analysisChecks if a loop can use hardware safe speculative reads for vectorization. As part of this establishes HSSR regions. A HSSR region is conceptually a set of HSSR loads, which can all be scheduled together, and a collection of SLP nodes and instances, which depend on the values from these loads, and so are "governed" by the HSSR region. That meaning their loop mask is the loop mask from the loop control anded with the mask of successfully loaded elements from the global HSSR state after the HSSR loads. The HSSR regions are stored in the vec_loop_masks struct as part of loop_vinfo. SLP instances and SLP trees both store an index for the region that governs them.This is a first pass, I probably need to go through it again, but some initial thoughts.
Thanks for the review! I really appreciate it.
gcc/ChangeLog: * tree-vect-data-refs.cc (vect_enhance_data_refs_alignment): Add logic to disable peeling when using HSSR. (vect_supportable_dr_alignment): Add logic that unaligned safe speculative loads are supported when using HSSR. * tree-vect-loop-manip.cc (vect_can_add_hssr_controls): New function. * tree-vect-loop.cc (_loop_vec_info::_loop_vec_info): Add new memebers must_use_hssr_p, can_use_hssr_p, using_hssr_p. (release_vec_loop_hssr_regions): New function. (_loop_vec_info::~_loop_vec_info): Add call to release_vec_loop_hssr_regions. (vect_verify_full_masking): Add call to vect_can_add_hssr_controls. (set_hssr_regions): New function. (sort_instances_by_dominators): New function. (vect_determine_hssr_and_versioning): New function. (vect_better_loop_vinfo_p): Add logic to prefer HSSR loops if vect_hssr_usage is 3. (vectorizable_induction): Add logic to disable HSSR for multilane inductions. (vect_get_hssr_region): New helper function. * tree-vect-slp.cc (_slp_tree::_slp_tree): Add hssr region index member. (vect_build_slp_instance): Likewise. (vect_analyze_slp_reduc_chain): Add initialization for hssr_region. (vect_analyze_slp_reduction): Likewise. (vect_analyze_slp_reduction_group): Likewise. (vect_analyze_slp_instance): Likewise. * tree-vect-stmts.cc (vect_hssr_load_ifn_for): New function. (get_load_store_type): Add logic to detect if hssr is possible for this load. (vectorizable_conversion): Add setting of SLP_TREE_HSSR_MASK_NEEDED to false when no fault could be caused by this SLP node. (vectorizable_load): Add logic to determine if HSSR can be used for this load, and to emit the correct load when it is required. * tree-vectorizer.cc (vec_info::new_stmt_vec_info): Add initalization for requires_hssr_p. * tree-vectorizer.h (struct hssr_region): New struct. (struct vect_load_store_data): Add hssr_ifn and defines_region members. (struct _slp_tree): Add hssr_region_index and hssr_mask_required members. (struct vec_loop_masks): Add (SLP_INSTANCE_HSSR_REGION): New macro. (SLP_TREE_HSSR_REGION): Likewise. (SLP_TREE_HSSR_MASK_NEEDED): Likewise. (LOOP_VINFO_CAN_USE_HSSR_P): Likewise. (LOOP_VINFO_MUST_USE_HSSR_P): Likewise. (LOOP_VINFO_USING_HSSR_P): Likewise. (add_stmt_cost): Add costing for HSSR regions. (vect_can_add_hssr_controls): New function. (vect_get_hssr_region): New function. --- gcc/tree-vect-data-refs.cc | 6 +- gcc/tree-vect-loop-manip.cc | 35 ++++ gcc/tree-vect-loop.cc | 322 ++++++++++++++++++++++++++++++++++++ gcc/tree-vect-slp.cc | 9 + gcc/tree-vect-stmts.cc | 140 +++++++++++++--- gcc/tree-vectorizer.cc | 1 + gcc/tree-vectorizer.h | 70 +++++++- 7 files changed, 560 insertions(+), 23 deletions(-) diff --git a/gcc/tree-vect-data-refs.cc b/gcc/tree-vect-data-refs.cc index 827e0965d9c..3b06175d82c 100644 --- a/gcc/tree-vect-data-refs.cc +++ b/gcc/tree-vect-data-refs.cc @@ -2532,7 +2532,8 @@ vect_enhance_data_refs_alignment (loop_vec_info loop_vinfo) /* We don't currently maintain the LCSSA for prologue peeled inversed loops. */ || (LOOP_VINFO_EARLY_BREAKS_VECT_PEELED (loop_vinfo) - && !LOOP_VINFO_NITERS_UNCOUNTED_P (loop_vinfo))) + && !LOOP_VINFO_NITERS_UNCOUNTED_P (loop_vinfo)) + || LOOP_VINFO_MUST_USE_HSSR_P (loop_vinfo)) do_peeling = false; struct _vect_peel_extended_info peel_for_known_alignment; @@ -6699,7 +6700,8 @@ vect_supportable_dr_alignment (vec_info *vinfo, dr_vec_info *dr_info, if (misalignment == 0) return dr_aligned; - else if (dr_safe_speculative_read_required (stmt_info)) + else if (dr_safe_speculative_read_required (stmt_info) + && !stmt_info->requires_hssr_p) return dr_unaligned_unsupported; if (loop_vinfo) diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc index 103657245a8..3147de99b5f 100644 --- a/gcc/tree-vect-loop-manip.cc +++ b/gcc/tree-vect-loop-manip.cc @@ -4918,3 +4918,38 @@ vect_loop_versioning (loop_vec_info loop_vinfo, return nloop; } + +/* Checks if it's possible to create the necessary controls for an HSSR loop. + + That is, we need to be able to generate all the required masks from permutes + of the successfully loaded element mask, which is the base mask. */ + +bool +vect_can_add_hssr_controls (loop_vec_info loop_vinfo)NIT, vect_can_add_hssr_controls_p?+{ + if (!LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo)) + return false; + + int i; + rgroup_controls *rgc; + FOR_EACH_VEC_ELT (LOOP_VINFO_MASKS (loop_vinfo).rgc_vec, i, rgc) + { + /* The base case is handled by reading the HSSR value. */ + if (i == 0) + continue; + + unsigned int nmasks = i + 1; + + rgroup_controls *half_controls_rgc + = &LOOP_VINFO_MASKS (loop_vinfo).rgc_vec[nmasks / 2 - 1]; + + if (!rgc->type) + return false; +Very dumb question, but is it correct to return early here vs continue? i.e. If the loop needs groups of 1, 2 and 4 masks, rgc_vec[2].type is null because no operation needs 3 masks and so we return here before checking the 4-mask group, which could be derived from the 2-mask group. Or am I misunderstanding?
Agh not a dumb question, just dumb code 🫠Yes you're right, I can continue here instead. Thanks, will change.
+ /* Permute the masks from the half rgoup to this one. */ + if (!vect_maybe_permute_loop_masks (NULL, rgc, half_controls_rgc)) + return false; + } + + return true; +} diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc index fdd224b50f9..e5c94125c7e 100644 --- a/gcc/tree-vect-loop.cc +++ b/gcc/tree-vect-loop.cc @@ -755,6 +755,9 @@ _loop_vec_info::_loop_vec_info (class loop *loop_in, vec_info_shared *shared) vectorizable (false), can_use_partial_vectors_p (true), must_use_partial_vectors_p (false), + must_use_hssr_p (false), + can_use_hssr_p (true), + using_hssr_p (false), using_partial_vectors_p (false), using_decrementing_iv_p (false), using_select_vl_p (false), @@ -844,6 +847,23 @@ release_vec_loop_controls (vec<rgroup_controls> *controls) controls->release (); } +/* Free all levels of hssr regions. */ + +voidAdd static+release_vec_loop_hssr_regions (vec<hssr_region> *regions) +{ + hssr_region *region; + unsigned int i; + FOR_EACH_VEC_ELT (*regions, i, region) + {Just use a normal for-each loop.+ for (auto &control : region->controls) + control.release (); + region->controls.release (); + region->loads.release (); + } + regions->release (); +} + /* Free all memory used by the _loop_vec_info, as well as all the stmt_vec_info structs of all the stmts in the loop. */ @@ -853,6 +873,7 @@ _loop_vec_info::~_loop_vec_info () release_vec_loop_controls (&masks.rgc_vec); release_vec_loop_controls (&lens); + release_vec_loop_hssr_regions (&masks.region_vec); delete ivexpr_map; delete scan_map; delete scalar_costs; @@ -1102,6 +1123,18 @@ vect_verify_full_masking (loop_vec_info loop_vinfo) LOOP_VINFO_RGROUP_COMPARE_TYPE (loop_vinfo) = cmp_type; LOOP_VINFO_RGROUP_IV_TYPE (loop_vinfo) = iv_type; LOOP_VINFO_PARTIAL_VECTORS_STYLE (loop_vinfo) = vect_partial_vectors_while_ult; + + /* Check we can generate all the masks we need for HSSR. */ + if (LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo) + && !vect_can_add_hssr_controls (loop_vinfo)) + { + LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo) = false; + if (dump_enabled_p ()) + dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, + "not using HSSR: Cannot generate the masks from the " + "HSSR mask.\n"); + } + return true; } @@ -2140,6 +2173,255 @@ vect_determine_partial_vectors_and_peeling (loop_vec_info loop_vinfo, return opt_result::success (); } +/* Sets the hssr region for NODE and its children to be REGION. */ + +static void +set_hssr_regions (loop_vec_info loop_vinfo, slp_tree node, unsigned new_region, + hash_set<slp_tree> &visited) +{ + /* All nodes start with region 0 by default. */ + if (new_region == 0) + return; + + /* If the node is an HSSR load, then return the region it defines. */ + if (!node || visited.add (node) || SLP_TREE_TYPE (node) == load_vec_info_type + || SLP_TREE_TYPE (node) == phi_info_type || SLP_TREE_HSSR_REGION (node)) + return; + + SLP_TREE_HSSR_REGION (node) + = SLP_TREE_HSSR_MASK_NEEDED (node) ? new_region : new_region - 1; + + /* Propagate the region to all unset children. */ + for (slp_tree child : SLP_TREE_CHILDREN (node)) + set_hssr_regions (loop_vinfo, child, SLP_TREE_HSSR_REGION (node), visited); +}So HSSR regions are globally unique over al instances right?+ +static int +sort_instances_by_dominators (const void *instance_1_, const void *instance_2_, + void *) +{ + const slp_instance *instance_1 = (const slp_instance*) instance_1_; + const slp_instance *instance_2 = (const slp_instance*) instance_2_; + + bool gcond_1_p = SLP_INSTANCE_KIND (*instance_1) == slp_inst_kind_gcond; + bool gcond_2_p = SLP_INSTANCE_KIND (*instance_2) == slp_inst_kind_gcond; + + /* If neither of the instances aree GCOND, then we don't care, and these + should all come at the end and use the last mask. */s/aree/are+ if (!gcond_1_p && !gcond_2_p) + return 0; + /* If only instance_2 is a GCOND, then that should be processed first. */ + else if (!gcond_1_p) + return 1; + /* If only instance_1 is a GCOND, then that should be processed first. */ + else if (!gcond_2_p) + return -1; + + stmt_vec_info stmt_info_1 + = SLP_TREE_REPRESENTATIVE (SLP_INSTANCE_TREE (*instance_1)); + stmt_vec_info stmt_info_2 + = SLP_TREE_REPRESENTATIVE (SLP_INSTANCE_TREE (*instance_2)); +The comments and the code make me think that you want to compare the gconds and not the statements feeding them. i.e. in t2 = .. (); t1 = .. (); if (t1) break; if (t2) break; t2 and t1 are the SLP_TREE_REPRESENTATIVE and in this case in opposite order of what you want. I assume you wanted SLP_INSTANCE_ROOT_STMTS (instance)[0] like in the scheduling code?
Oof thank you for pointing this out! I completely misunderstood, thank you! I suppose I got lucky (or unlucky?) in my testing
+ /* If both instances are GCONDS, then they should be ordered why which + statements BB dominates the other. */ + if (vect_stmt_dominates_stmt_p (stmt_info_1->stmt, stmt_info_2->stmt)) + return -1; + else if (vect_stmt_dominates_stmt_p (stmt_info_2->stmt, stmt_info_1-stmt))+ return 1; + return 0; +} + +/* Checks if hssr loads can be used for this loop and makes a + decision enabling them. + + Establishes hssr-regions. */ + +static opt_result +vect_determine_hssr_and_versioning (loop_vec_info loop_vinfo) +{ + vec<slp_instance> instances; + hssr_region *new_region = NULL; + slp_instance instance; + int i; + hash_set<slp_tree> visited; + unsigned current_hssr_region_index; + + if (!LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo)) + goto exit; + + if (param_vect_hssr_usage == 0) + { + if (dump_enabled_p ()) + dump_printf_loc (MSG_NOTE, vect_location, "HSSR: Disabled.\n"); + LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo) = false; + goto exit; + } + + if (!LOOP_VINFO_FULLY_MASKED_P (loop_vinfo)) + { + if (dump_enabled_p ()) + dump_printf_loc (MSG_NOTE, vect_location, + "HSSR: Loop not fully masked.\n"); + LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo) = false; + goto exit; + } + + if (param_vect_hssr_usage != 3 + && !LOOP_VINFO_MUST_USE_HSSR_P (loop_vinfo) + && !LOOP_REQUIRES_VERSIONING_FOR_ALIGNMENT (loop_vinfo)) + { + if (dump_enabled_p ()) + dump_printf_loc (MSG_NOTE, vect_location, + "HSSR: There is no reason to.\n"); + LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo) = false; + goto exit; + } + + /* If any of the SLP instances cover more than a single lane + we cannot use HSSR at the moment, even if the number + of lanes is uniform throughout the SLP graph. */ + for (slp_instance inst : LOOP_VINFO_SLP_INSTANCES (loop_vinfo)) + if (SLP_TREE_LANES (SLP_INSTANCE_TREE (inst)) != 1 + && !(SLP_INSTANCE_KIND (inst) == slp_inst_kind_store + && SLP_INSTANCE_TREE (inst)->ldst_lanes)) + { + if (dump_enabled_p ()) + dump_printf_loc (MSG_NOTE, vect_location, + "HSSR: Can't handle multiple lanes.\n"); + LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo) = false; + goto exit; + }I'm not sure I understand this limitation Early break blocks are limited atm single lanes sure, but I don't see why multiple lanes would be a problem? I can imagine you not supporting vect_get_num_copies () != 1, but lanes seems odd. Especially as you put the same restriction on non-early break blocks, and I guess the exclusion of slp stores and ldst_lanes are for cases like For instance, https://godbolt.org/z/jdfrxd4oW void f (unsigned int *__restrict out, const unsigned short *__restrict in, int n) { for (int i = 0; i < n; i++) { if (in[i] > 100) return; out[i*2] = in[i] + 1; out[i*2+1] = in[i] + 2; } } It seems like an odd place to reject loads needing HSSR, would this not be better in vectorizable_load? Since this is called after vect_analyze_slp.
Again I think you're right and this is overly restrictive. I wasn't aware of the restriction on early break but that totally makes sense.
This was bourn out of disallowing multi-lane HSSR loads, as we cannot deal with potentially needing to rotate any loop invariant vectors if we don't load a full iteration (and we have issues with forward progress).
But now that I understand how the SLP tree works more and the vectorizer you're completely right that this is covered by the restriction on the HSSR loads, and this is overly restrictive. Will fix!
+ + /* Sort the instances by their gcond domination. */ + + instances = LOOP_VINFO_SLP_INSTANCES (loop_vinfo).copy (); + instances.stablesort (sort_instances_by_dominators, NULL); + + current_hssr_region_index = 0;Aha, so it is globally unique.+ /* Work out the HSSR regions. */ + FOR_EACH_VEC_ELT (instances, i, instance) + { + /* Check if the instance needs an HSSR region. + + NOTE: This assumes all HSSR loads are leaf nodes. + + This is a safe assumption as HSSR loads can't sensibly be made to + do conditional loads so can never take a mask, and we don't yet + support gather HSSR loads. */ + if (SLP_INSTANCE_KIND (instance) == slp_inst_kind_gcond) + { + LOOP_VINFO_MASKS (loop_vinfo) + .region_vec.safe_grow_cleared (current_hssr_region_index + 1, true); + new_region = &LOOP_VINFO_MASKS (loop_vinfo) + .region_vec[current_hssr_region_index]; + for (slp_tree load : SLP_INSTANCE_LOADS (instance)) + { + gcc_assert (load->children.length () == 0); + + /* Set the load "defines region" to this one. */ + vect_load_store_data _ls_data{}; + vect_load_store_data &ls = load->get_data (_ls_data); + + stmt_vec_info stmt_info = SLP_TREE_REPRESENTATIVE (load); + if (!dr_safe_speculative_read_required (stmt_info)) + continue; + + const vect_memory_access_type memory_access_type + = ls.memory_access_type; + if (memory_access_type == VMAT_INVARIANT) + continue; + + /* The number of elements of the load must equal the + vectorization factor, as we must only fault if the next + scalar iteration is a fault, and optionally load later + iterations. + + This is checked by vectoriable_load. + + TODO: We could support other cases and grain sizes here. + However, this would likely require separate HSSR regions + which would have high runtime cost. */ + gcc_assert (known_eq (GET_MODE_NUNITS (TYPE_MODE (load-vectype)),+ LOOP_VINFO_VECT_FACTOR (loop_vinfo))); + + /* If this load is already handled by another region, we don't + need to handle it in this one. */ + if (ls.defines_region) + continue; + + /* This load will define the new region. */ + ls.defines_region = current_hssr_region_index + 1; + + /* This load will be part of the region for this break. */ + new_region->loads.safe_push (load); + new_region->prev_region = current_hssr_region_index; + + /* Set the region it uses for its mask to be the previous one. + */ + SLP_TREE_HSSR_REGION (load) = current_hssr_region_index; + + /* Set the read point to be the last load. */ + if (!new_region->hssr_read_point + || vect_stmt_dominates_stmt_p + (new_region->hssr_read_point, + SLP_TREE_REPRESENTATIVE (load)->stmt)) + new_region->hssr_read_point + = SLP_TREE_REPRESENTATIVE (load)->stmt; + } + + /* If this region doesnt handle any loads, it isn't needed. */ + if (new_region->loads.is_empty ()) + LOOP_VINFO_MASKS (loop_vinfo).region_vec.pop (); + else + current_hssr_region_index += 1; + + /* Instances region should be the previous instance. */ + SLP_INSTANCE_HSSR_REGION (instance) = current_hssr_region_index; + } + + /* Populate the hssr_regions for the nodes in the tree. */ + set_hssr_regions (loop_vinfo, SLP_INSTANCE_TREE (instance), + current_hssr_region_index, visited); + } + + /* If none of the instances require loads, then HSSR is pointless. */ + if (current_hssr_region_index == 0) + { + LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo) = false; + if (dump_enabled_p ()) + dump_printf_loc (MSG_NOTE, vect_location, + "HSSR: No early break instances require HSSR.\n"); + goto exit; + } + + /* We can use HSSR. */ + if (dump_enabled_p ()) + dump_printf_loc (MSG_NOTE, vect_location, "HSSR: Will use HSSR\n"); + + /* Can and will use HSSR. */ + LOOP_VINFO_MUST_USE_PARTIAL_VECTORS_P (loop_vinfo) = true; + LOOP_VINFO_USING_HSSR_P (loop_vinfo) = true; + + /* We do not need alignment for safety with HSSR. */ + LOOP_VINFO_MAY_MISALIGN_STMTS (loop_vinfo) = vNULL; + LOOP_VINFO_ALLOW_MUTUAL_ALIGNMENT (loop_vinfo) = false; + /* With HSSR we can have unlimited speculative read. */ + LOOP_VINFO_MAX_SPEC_READ_AMOUNT (loop_vinfo) = 0; + /* There is no reason to peel for alignment as with HSSR loop variant step + it won't stay aligned. Alignment is only used for safe speculative reads + so there should never be needed with HSSR anyway. */ + LOOP_VINFO_PEELING_FOR_ALIGNMENT (loop_vinfo) = 0;I'm assuming that this works costing wise because vectorizable_load will handle cases like known fixed sizes where raising the alignment is cheaper?
Hmm unfortunately my HSSR costing is really poor, and I would love advice for how to improve it.
I think you're already clear that, for HSSR, trying to achieve any alignment is pointless, because you will lose it as soon as you get an incomplete load.
So I think you are worried that we may have costed a load cheaply, because we think we could do the alignment trick, but when we here choose to do HSSR, then we can no longer do that?
And that is an issue that I have no fix for that right now and I think it is a flaw of the code.
In general the HSSR analysis flow is something I've found really challenging. I am not sure how to deal with the fact that we can't decide whether to use HSSR until we have completed analysis and know about safety. But obviously costing happens at the same time as we work out if it would be possible to use it.
Additionally, there are possible requirements for HSSR that could cause us to want to do analysis/SLP tree construction differently, like forcing single lanes for the loads, and such.
This patch kind of get's away with this (at least at -Ofast/-O3) as we only use HSSR with versioning, so for the second loop (the HSSR one) we force HSSR on from the beginning, so we can accurately cost HSSR, as we will either use it, or not vectorize, and keep the scalar fallback.
But for -O2, where we use HSSR whenever we need safe speculative reads and can't guarantee getting alignment, then there is no such fix. As we only discover that we require HSSR at some point during analysis.
I had two other ideas that seemed too invasive/complicated to be worth it?1. Instead of deciding on HSSR within vect_analyze_loop_2 (when working with a specific VF mode), change to treating HSSR as another "VF mode", by which I mean, something that is passed into vect_analyze_loop_2 as an argument.
Then initiall we analyze a loop without HSSR, but record if HSSR is possible and could be useful, and, then if it is, completely re-run vect_analyze_loop_2, but with HSSR turned on, to get how costly it would be.
We then could do all the costing logic required, such as determining if we should: use HSSR only, version for a mututally aligned loop and HSSR loop, or not use HSSR at all and version to use a scalar fallback instead.
This would also hugely simplify some of the hacks I did to make HSSR versioning work (which I think needs to be changed anyway)
2. Alternatively, within vect_analyze_loop_2, we could maintain two copies of costs, one for the current costing model, and one for a best effort guess at how much HSSR would cost. Then similarly, in vect_analyze_loop_1 we could make the same decision.
I think option 2 would have less compile time impact, but would be messier and would be more "compromised" in that we might miss some more complex aspects of HSSR costs.
But maybe both are too weird? Hopefully that makes sense.I would love your thoughts/advice. I am aware I may be missing something obvious here.
+ +exit: + if (LOOP_VINFO_MUST_USE_HSSR_P (loop_vinfo) + && !LOOP_VINFO_USING_HSSR_P (loop_vinfo)) + return opt_result::failure_at (vect_location, + "not vectorized: loop needs but cannot " + "use hssr\n"); + + return opt_result::success (); +} + /* Function vect_analyze_loop_2. Apply a set of analyses on LOOP specified by LOOP_VINFO, the different @@ -2409,6 +2691,11 @@ start_over: if (!ok) return ok; + /* Check if we can use HSSR instead of peeling and/or versioning. */ + ok = vect_determine_hssr_and_versioning (loop_vinfo); + if (!ok) + return ok; + /* If we're vectorizing a loop that uses length "controls" and can iterate more than once, we apply decrementing IV approach in loop control. */ @@ -2723,6 +3010,7 @@ again: LOOP_VINFO_MASKS (loop_vinfo).mask_set.empty (); release_vec_loop_controls (&LOOP_VINFO_MASKS (loop_vinfo).rgc_vec); release_vec_loop_controls (&LOOP_VINFO_LENS (loop_vinfo)); + release_vec_loop_hssr_regions (&LOOP_VINFO_MASKS (loop_vinfo).region_vec); /* Reset assorted flags. */ LOOP_VINFO_PEELING_FOR_NITER (loop_vinfo) = false; LOOP_VINFO_PEELING_FOR_GAPS (loop_vinfo) = false; @@ -2731,6 +3019,9 @@ again: LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P (loop_vinfo) = saved_can_use_partial_vectors_p; LOOP_VINFO_MUST_USE_PARTIAL_VECTORS_P (loop_vinfo) = false; + LOOP_VINFO_USING_HSSR_P (loop_vinfo) = false; + LOOP_VINFO_MUST_USE_HSSR_P (loop_vinfo) = false; + LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo) = true; LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo) = false; LOOP_VINFO_USING_SELECT_VL_P (loop_vinfo) = false; LOOP_VINFO_USING_DECREMENTING_IV_P (loop_vinfo) = false; @@ -2772,6 +3063,19 @@ vect_better_loop_vinfo_p (loop_vec_info new_loop_vinfo, if (loop_vec_info main_loop = LOOP_VINFO_ORIG_LOOP_INFO (old_loop_vinfo)) return new_costs->better_epilogue_loop_than_p (old_costs, main_loop); + /* If we should force HSSR when it is possible. Then always prefer the loops + where it is possible. */ + + if ((LOOP_VINFO_USING_HSSR_P (new_loop_vinfo) + != LOOP_VINFO_USING_HSSR_P (old_loop_vinfo)) + && param_vect_hssr_usage == 3) + { + if (dump_enabled_p ()) + dump_printf_loc (MSG_NOTE, vect_location, + "HSSR: Preferring loop with HSSR\n"); + return LOOP_VINFO_USING_HSSR_P (new_loop_vinfo); + } + return new_costs->better_main_loop_than_p (old_costs); } @@ -9630,6 +9934,15 @@ vectorizable_induction (loop_vec_info loop_vinfo, return false; } + if (SLP_TREE_LANES (slp_node) != 1) + { + if (dump_enabled_p ()) + dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, + "HSSR: Cannot use HSSR as multiple-lane SLP inductions" + "aren't supported.\n"); + LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo) = false; + }I still wonder whether your condition shouldn't be if (nvects != 1). i.e. cases where you need more than one vector statement in the output.
I am pretty sure we need both SLP_TREE_LANES == 1 and nvects == 1 for HSSR.The issue is we require the resulting HSSR mask to have each element correspond to *exactly* one scalar iteration of the original loop.
This is to guarantee we don't split scalar iterations, and so that we keep the guarantee of forward progress.
We could allow situations where SLP_TREE_LANES > 1, if we did some trick such as treating a situation like a load with 2 lanes, each of which bytes, as instead a load of 16bit elements. So the HSSR mask corresponds still to 1 element per scalar iteration. (though we would have to be careful about element alignment I think)
Additionally, we could support a situation such as SLP_TREE_LANES = 2 and nvects = 2 by instead of doing continuous loads, doing gather loads (or strided loads) of the two sets of elements, and then zipping them together.
And lastly, we could just support situations where nvects > 1, and SLP_TREE_LANES = 1, by having separate regions for the first lane, and the second lane, the second lane using non-faulting loads, and permuting the resulting masks together into one result mask.
All of this is not things I am going to tackle right now. But hopefully demonstrates why we need the condition that SLP_TREE_LANES == 1 and nvects == 1 at the moment, and why HSSR may (in the future) change the way we construct the SLP tree itself, which may prove complex and have a lot more impacts on costing.
Same question for the use below. I'll get to the remaining two tomorrow. But this looks pretty good so far! Thanks for working on this, Tamar
Thank you so much for the reviews, sorry for the absolute essay!The other feedback you gave here I competely agree with, and will change for the next version.
KR, Alfie>
+ if (FLOAT_TYPE_P (vectype) && !param_vect_induction_float) { if (dump_enabled_p ()) @@ -10846,6 +11159,15 @@ vect_get_loop_len (loop_vec_info loop_vinfo, gimple_stmt_iterator *gsi, return loop_len; } +hssr_region * +vect_get_hssr_region (loop_vec_info loop_vinfo, unsigned region_index) +{ + if (region_index == 0) + return NULL; + /* We index from 1 for regions, as 0 is considered the loop masks. */ + return &LOOP_VINFO_MASKS (loop_vinfo).region_vec[region_index - 1]; +} + /* Generate the tree for the loop len mask and return it. Given the lens, nvectors, vectype, index and factor to gen the len mask as below. diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc index e0a32ade79a..81409a6a9ab 100644 --- a/gcc/tree-vect-slp.cc +++ b/gcc/tree-vect-slp.cc @@ -135,6 +135,8 @@ _slp_tree::_slp_tree () SLP_TREE_TYPE (this) = undef_vec_info_type; this->data = NULL; this->si = NULL; + SLP_TREE_HSSR_REGION (this) = 0; + SLP_TREE_HSSR_MASK_NEEDED (this) = true; } /* Tear down a SLP node. */ @@ -4233,6 +4235,7 @@ vect_build_slp_instance (vec_info *vinfo, new_instance->reduc_phis = NULL; new_instance->cost_vec = vNULL; new_instance->subgraph_entries = vNULL; + SLP_INSTANCE_HSSR_REGION (new_instance) = 0; if (dump_enabled_p ()) dump_printf_loc (MSG_NOTE, vect_location, @@ -4605,6 +4608,7 @@ vect_analyze_slp_reduc_chain (loop_vec_info vinfo, new_instance->reduc_phis = NULL; new_instance->cost_vec = vNULL; new_instance->subgraph_entries = vNULL; + SLP_INSTANCE_HSSR_REGION (new_instance) = 0; vinfo->slp_instances.safe_push (new_instance); @@ -4669,6 +4673,7 @@ vect_analyze_slp_reduc_chain (loop_vec_info vinfo, new_instance->reduc_phis = NULL; new_instance->cost_vec = vNULL; new_instance->subgraph_entries = vNULL; + SLP_INSTANCE_HSSR_REGION (new_instance) = 0; vect_reduc_info reduc_info = info_for_reduction (vinfo, node); reduc_info->is_reduc_chain = true; @@ -4807,6 +4812,7 @@ vect_analyze_slp_reduction (loop_vec_info vinfo, new_instance->reduc_phis = NULL; new_instance->cost_vec = vNULL; new_instance->subgraph_entries = vNULL; + SLP_INSTANCE_HSSR_REGION (new_instance) = 0; if (dump_enabled_p ()) dump_printf_loc (MSG_NOTE, vect_location, @@ -4877,6 +4883,7 @@ vect_analyze_slp_reduction_group (loop_vec_info loop_vinfo, new_instance->reduc_phis = NULL; new_instance->cost_vec = vNULL; new_instance->subgraph_entries = vNULL; + SLP_INSTANCE_HSSR_REGION (new_instance) = 0; if (dump_enabled_p ()) dump_printf_loc (MSG_NOTE, vect_location, @@ -5101,6 +5108,7 @@ vect_analyze_slp_instance (vec_info *vinfo, new_instance->reduc_phis = NULL; new_instance->cost_vec = vNULL; new_instance->subgraph_entries = vNULL; + SLP_INSTANCE_HSSR_REGION (new_instance) = 0; if (dump_enabled_p ()) dump_printf_loc (MSG_NOTE, vect_location, @@ -5340,6 +5348,7 @@ vect_analyze_slp_instance (vec_info *vinfo, new_instance->reduc_phis = NULL; new_instance->cost_vec = vNULL; new_instance->subgraph_entries = vNULL; + SLP_INSTANCE_HSSR_REGION (new_instance) = 0; if (dump_enabled_p ()) dump_printf_loc (MSG_NOTE, vect_location, diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc index 8cd5fb2001e..0fd2d5a4f0f 100644 --- a/gcc/tree-vect-stmts.cc +++ b/gcc/tree-vect-stmts.cc @@ -2051,6 +2051,21 @@ has_consecutive_load_permutation (slp_tree node) return vect_load_perm_consecutive_p (node); } +/* If the load represented by LS can be a HSSR load, then return the internal + function for the respective HSSR load. Otherwise, return IFN_LAST. */ + +static internal_fn +vect_hssr_load_ifn_for (vect_load_store_data *ls) +{ + switch (ls->memory_access_type) + { + case VMAT_CONTIGUOUS: + return IFN_MASK_FF_HSSR_LOAD; + + default: + return IFN_LAST; + } +} /* Analyze load or store SLP_NODE of type VLS_TYPE. Return true if there is a memory access type that the vectorized form can use, @@ -2532,6 +2547,29 @@ get_load_store_type (vec_info *vinfo, stmt_vec_info stmt_info, } } + /* Now that the VMAT is decided, check if we can support HSSR for this + load. */ + if (loop_vinfo + && LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo) + && dr_safe_speculative_read_required (stmt_info) + && *memory_access_type != VMAT_INVARIANT) + { + tree mask_type = truth_type_for (vectype); + + internal_fn ifn = vect_hssr_load_ifn_for (ls); + if (ifn == IFN_LAST + || !direct_internal_fn_supported_p (ifn, vectype, mask_type, + OPTIMIZE_FOR_SPEED)) + { + if (dump_enabled_p ()) + dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, + "HSSR: Load does not support HSSR\n"); + LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo) = false; + } + else + ls->hssr_ifn = ifn; + } + if (overrun_p) { gcc_assert (can_overrun_p); @@ -2575,6 +2613,8 @@ get_load_store_type (vec_info *vinfo, stmt_vec_info stmt_info, STMT_VINFO_STMT (stmt_info)); if (inbounds) LOOP_VINFO_MUST_USE_PARTIAL_VECTORS_P (loop_vinfo) = true; + else if (loop_vinfo && LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo)) + stmt_info->requires_hssr_p = true; else return false; } @@ -2608,16 +2648,25 @@ get_load_store_type (vec_info *vinfo, stmt_vec_info stmt_info, = DR_TARGET_ALIGNMENT (STMT_VINFO_DR_INFO (stmt_info)); if (!multiple_p (target_alignment, read_amount)) { - if (dump_enabled_p ()) + if (loop_vinfo && LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo)) { - dump_printf_loc (MSG_NOTE, vect_location, - "desired alignment not met, target was "); - dump_dec (MSG_NOTE, target_alignment); - dump_printf (MSG_NOTE, " previously, but read amount is "); - dump_dec (MSG_NOTE, read_amount); - dump_printf (MSG_NOTE, " at %G.\n", STMT_VINFO_STMT (stmt_info)); + stmt_info->requires_hssr_p = true; + *alignment_support_scheme = dr_unaligned_supported; + } + else + { + if (dump_enabled_p ()) + { + dump_printf_loc (MSG_NOTE, vect_location, + "desired alignment not met, target was "); + dump_dec (MSG_NOTE, target_alignment); + dump_printf (MSG_NOTE, " previously, but read amount is "); + dump_dec (MSG_NOTE, read_amount); + dump_printf (MSG_NOTE, " at %G.\n", + STMT_VINFO_STMT (stmt_info)); + } + return false; } - return false; } /* When using a group access the first element may be aligned but the @@ -2644,14 +2693,19 @@ get_load_store_type (vec_info *vinfo, stmt_vec_info stmt_info, exceeds the min page size. */ if (known_gt (read_amount, (unsigned) param_min_pagesize)) { - if (dump_enabled_p ()) + if (loop_vinfo && LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo)) + stmt_info->requires_hssr_p = true; + else { - dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, - "alignment required for correctness ("); - dump_dec (MSG_MISSED_OPTIMIZATION, read_amount); - dump_printf (MSG_NOTE, ") may exceed page size.\n"); + if (dump_enabled_p ()) + { + dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, + "alignment required for correctness ("); + dump_dec (MSG_MISSED_OPTIMIZATION, read_amount); + dump_printf (MSG_NOTE, ") may exceed page size.\n"); + } + return false; } - return false; } if (!vf.is_constant ()) @@ -2670,10 +2724,27 @@ get_load_store_type (vec_info *vinfo, stmt_vec_info stmt_info, if (*alignment_support_scheme == dr_unaligned_unsupported) { - if (dump_enabled_p ()) - dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, - "unsupported unaligned access\n"); - return false; + /* Can try getting safety by reading HSSR. */ + if (loop_vinfo && LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo)) + { + stmt_info->requires_hssr_p = true; + LOOP_VINFO_MUST_USE_HSSR_P (loop_vinfo) = true; + } + else + { + if (dump_enabled_p ()) + dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, + "unsupported unaligned access\n"); + return false; + } + } + + if (stmt_info->requires_hssr_p) + { + gcc_assert (loop_vinfo); + *alignment_support_scheme = dr_unaligned_supported; + LOOP_VINFO_MUST_USE_HSSR_P (loop_vinfo) = true; + LOOP_VINFO_MUST_USE_PARTIAL_VECTORS_P (loop_vinfo) = true; } /* FIXME: At the moment the cost model seems to underestimate the @@ -5686,6 +5757,14 @@ vectorizable_conversion (vec_info *vinfo, if (cost_vec) /* transformation not required. */ { + /* If this is not a conversion that could trap, then we do not require + a totally accurate HSSR mask. + + This allows conversions to be emitted next to loads and be combined + into widening loads. */ + if (!gimple_could_trap_p (stmt)) + SLP_TREE_HSSR_MASK_NEEDED (slp_node) = false; + if (!vect_maybe_update_slp_op_vectype (slp_op0, vectype_in) || !vect_maybe_update_slp_op_vectype (slp_op1, vectype_in)) { @@ -9983,8 +10062,29 @@ vectorizable_load (vec_info *vinfo, return false; } - if (dump_enabled_p () - && memory_access_type != VMAT_ELEMENTWISE + /* If we can currently do HSSR. */ + if (loop_vinfo && LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo)) + { + if (dr_safe_speculative_read_required (stmt_info) + && (vec_num != 1 || slp_node->lanes != 1)) + { + /* For now, dont handle group_size != 1 or vec_num != 1. */ + LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo) = false; + if (dump_enabled_p ()) + dump_printf_loc ( + MSG_MISSED_OPTIMIZATION, vect_location, + "HSSR: multiple lane loads aren't supported\n"); + } + if (!LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P (loop_vinfo)) + { + LOOP_VINFO_CAN_USE_HSSR_P (loop_vinfo) = false; + if (dump_enabled_p ()) + dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, + "HSSR: partial vectors are required\n"); + } + } + + if (dump_enabled_p () && memory_access_type != VMAT_ELEMENTWISE && !mat_gather_scatter_p (memory_access_type) && memory_access_type != VMAT_STRIDED_SLP && memory_access_type != VMAT_INVARIANT diff --git a/gcc/tree-vectorizer.cc b/gcc/tree-vectorizer.cc index 398edbd7ecc..460905aa9ff 100644 --- a/gcc/tree-vectorizer.cc +++ b/gcc/tree-vectorizer.cc @@ -741,6 +741,7 @@ vec_info::new_stmt_vec_info (gimple *stmt) /* This is really "uninitialized" until vect_compute_data_ref_alignment. */ res->dr_aux.misalignment = DR_MISALIGNMENT_UNINITIALIZED; + res->requires_hssr_p = false; return res; } diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h index 7ce13828c0a..272703c1d17 100644 --- a/gcc/tree-vectorizer.h +++ b/gcc/tree-vectorizer.h @@ -276,6 +276,31 @@ struct vect_simd_clone_data : vect_data { auto_vec<tree> simd_clone_info; }; +struct hssr_region { + hssr_region () = default; + + /* The HSSR loads that create the mask that defines this region. */ + vec<slp_tree> loads; + + /* Where the HSSR and the fixup code for this region read should be + inserted. */ + gimple* hssr_read_point; + + /* The SSA node to store how much IV's should be incremented by. */ + tree num_iter; + + /* The SSA node for the mask of elements not processed by this vectorized + iteration that will need to be re-processed. */ + tree next_iter_mask; + + /* The SSA nodes for the masks that operations in this region will use. */ + vec<vec<tree>> controls; + + /* The index of the region before this one, or 0 if this is the first + region. */ + unsigned prev_region; +}; + /* Analysis data from vectorizable_load and vectorizable_store for load_vec_info_type and store_vec_info_type. */ struct vect_load_store_data : vect_data { @@ -287,6 +312,7 @@ struct vect_load_store_data : vect_data { dr_alignment_support alignment_support_scheme; int misalignment; internal_fn lanes_ifn; // VMAT_LOAD_STORE_LANES + internal_fn hssr_ifn; poly_int64 poffset; union { internal_fn ifn; // VMAT_GATHER_SCATTER_IFN @@ -314,6 +340,10 @@ struct vect_load_store_data : vect_data { unsigned n_loads; // SLP_TREE_LOAD_PERMUTATION /* Whether the load permutation is consecutive and simple. */ bool subchain_p; // VMAT_STRIDED_SLP and VMAT_GATHER_SCATTER + + /* If this is a hssr load, then the region that the resultant + mask will define. 0 for does not define a region. */ + unsigned defines_region; }; /* A computation tree of an SLP instance. Each node corresponds to a group of @@ -411,6 +441,14 @@ struct _slp_tree { /* Linked list of nodes to release when we free the slp_tree_pool. */ slp_tree next_node; slp_tree prev_node; + + /* The hssr region this node is goverened by and gets it's mask from. + Will be 0 if there is no hssr region governing this node and it should use + the loop masks. */ + unsigned hssr_region; + /* If this HSSR node needs to have an accurate HSSR mask, or can make do with + a possibly inaccurate superset mask. */ + bool hssr_mask_needed; }; /* The enum describes the type of operations that an SLP instance @@ -456,6 +494,18 @@ public: /* The type of operation the SLP instance is performing. */ slp_instance_kind kind; + /* The hssr region defining the mask for the result of the SLP instance. 0 + if the instances result is not governed by an HSSR region. + + Note that the governing region for the HSSR region does _not_ need to be + the same at the root node of the HSSR region, as the result node may occur + in other instances and therefor may occur first in other HSSR regions. + + However, the HSSR region for the instance will necessarily be a later + region that the HSSR region for the node, and so is always the same mask + or smaller. */ + unsigned hssr_region; + dump_user_location_t location () const; } *slp_instance; @@ -466,6 +516,7 @@ public: #define SLP_INSTANCE_ROOT_STMTS(S) (S)->root_stmts #define SLP_INSTANCE_REMAIN_DEFS(S) (S)->remain_defs #define SLP_INSTANCE_KIND(S) (S)->kind +#define SLP_INSTANCE_HSSR_REGION(S) (S)->hssr_region #define SLP_TREE_CHILDREN(S) (S)->children #define SLP_TREE_SCALAR_STMTS(S) (S)->stmts @@ -485,6 +536,8 @@ public: #define SLP_TREE_GS_BASE(S) (S)->gs_base #define SLP_TREE_REDUC_IDX(S) (S)-cycle_info.reduc_idx#define SLP_TREE_PERMUTE_P(S) ((S)->code == VEC_PERM_EXPR) +#define SLP_TREE_HSSR_REGION(S) (S)->hssr_region +#define SLP_TREE_HSSR_MASK_NEEDED(S) (S)-hssr_mask_neededinline vect_memory_access_type SLP_TREE_MEMORY_ACCESS_TYPE (slp_tree node) @@ -846,6 +899,9 @@ struct vec_loop_masks /* rgroup_controls used for the partial vector scheme. */ auto_vec<rgroup_controls> rgc_vec; + + /* The HSSR regions (if using HSSR). */ + auto_vec<hssr_region> region_vec; }; typedef auto_vec<rgroup_controls> vec_loop_lens; @@ -1146,6 +1202,10 @@ public: /* Records whether we must use niter masking for correctness reasons. */ bool must_use_partial_vectors_p; + bool must_use_hssr_p; + bool can_use_hssr_p; + bool using_hssr_p; + /* True if we've decided to use partially-populated vectors, so that the vector loop can handle fewer than VF scalars. */ bool using_partial_vectors_p; @@ -1307,6 +1367,9 @@ public: #define LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P(L) (L)-can_use_partial_vectors_p#define LOOP_VINFO_MUST_USE_PARTIAL_VECTORS_P(L) (L)-must_use_partial_vectors_p#define LOOP_VINFO_USING_PARTIAL_VECTORS_P(L) (L)-using_partial_vectors_p+#define LOOP_VINFO_CAN_USE_HSSR_P(L) (L)->can_use_hssr_p +#define LOOP_VINFO_MUST_USE_HSSR_P(L) (L)->must_use_hssr_p +#define LOOP_VINFO_USING_HSSR_P(L) (L)->using_hssr_p #define LOOP_VINFO_USING_DECREMENTING_IV_P(L) (L)-using_decrementing_iv_p#define LOOP_VINFO_USING_SELECT_VL_P(L) (L)->using_select_vl_p #define LOOP_VINFO_ALLOW_MUTUAL_ALIGNMENT(L) (L)-allow_mutual_alignment@@ -1672,6 +1735,9 @@ public: /* True if this is only suitable for SLP vectorization. */ bool slp_vect_only_p; + + /* This node requires using HSSR for safety. */ + bool requires_hssr_p; }; /* Information about a gather/scatter call. */ @@ -2138,7 +2204,7 @@ add_stmt_cost (vector_costs *costs, int count, enum vect_cost_for_stmt kind, enum vect_cost_model_location where) { gcc_assert (kind == cond_branch_taken || kind == cond_branch_not_taken - || kind == scalar_stmt); + || kind == scalar_stmt || kind == hssr_read); return add_stmt_cost (costs, count, kind, NULL, NULL, NULL_TREE, 0, where); } @@ -2488,6 +2554,7 @@ class loop *slpeel_tree_duplicate_loop_to_edge_cfg (class loop *, edge, bool = false, bool = false, bool = true); class loop *vect_loop_versioning (loop_vec_info, gimple *); +extern bool vect_can_add_hssr_controls (loop_vec_info); extern class loop *vect_do_peeling (loop_vec_info, tree, tree, tree *, tree *, tree *, int, bool, bool, tree *); @@ -2684,6 +2751,7 @@ extern void vect_record_loop_mask (loop_vec_info, vec_loop_masks *, extern tree vect_get_loop_mask (loop_vec_info, gimple_stmt_iterator *, vec_loop_masks *, unsigned int, tree, unsigned int); +extern hssr_region *vect_get_hssr_region (loop_vec_info, unsigned); extern void vect_record_loop_len (loop_vec_info, vec_loop_lens *, unsigned int, tree, unsigned int); extern tree vect_get_loop_len (loop_vec_info, gimple_stmt_iterator *, -- 2.43.0
