Hi Sandra,
given that it is "just" a revised patch of previously posted patch and
(code wise not file wise) very localized to OpenMP code - and even there
touching only 'declare ...' code in a simple way:
I would be still fine to have it in GCC 14, but I want to give Jakub a
chance to shout...
Glancing at the code, it looks okayish, but I have two quick comments.
(I have now an errant to do - and will continue later with the review.)
Sandra Loosemore wrote:
+static tree
+omp_dynamic_cond (tree ctx)
+{
+ tree expr = NULL_TREE;
+
+ tree user = omp_get_context_selector (ctx, OMP_TRAIT_SET_USER,
+ OMP_TRAIT_USER_CONDITION);
+ if (user)
+ {
+ tree expr_list = OMP_TS_PROPERTIES (user);
+
+ gcc_assert (OMP_TP_NAME (expr_list) == NULL_TREE);
+
+ /* The user condition is not dynamic if it is constant. */
+ if (!tree_fits_shwi_p (TREE_VALUE (expr_list)))
+ expr = TREE_VALUE (expr_list);
+ }
+
+ tree target_device
+ = omp_get_context_selector_list (ctx, OMP_TRAIT_SET_TARGET_DEVICE);
+ if (target_device)
+ {
+ tree device_num = null_pointer_node;
+ tree kind = null_pointer_node;
+ tree arch = null_pointer_node;
+ tree isa = null_pointer_node;
+
+ tree device_num_sel
+ = omp_get_context_selector (ctx, OMP_TRAIT_SET_TARGET_DEVICE,
+ OMP_TRAIT_DEVICE_NUM);
+ if (device_num_sel)
+ device_num = OMP_TP_VALUE (OMP_TS_PROPERTIES (device_num_sel));
+ else
+ device_num = build_int_cst (integer_type_node, -1);
I think this won't correctly distinguish 'omp_initial_device' (= -1)
from unset. OpenMP has 'omp_invalid_device', 'omp_initial_device' and
then the values 0 to omp_get_num_devices() [where the latter denotes
the host, 0 ... < omp_get_num_device() are non-host devices].
/* Compute *SCORE for context selector CTX. Return true if the score
would be different depending on whether it is a declare simd clone or
@@ -2194,12 +2308,21 @@ omp_context_compute_score (tree ctx, score_wide_int
*score, bool declare_simd)
{
tree selectors
= omp_get_context_selector_list (ctx, OMP_TRAIT_SET_CONSTRUCT);
- bool has_kind = omp_get_context_selector (ctx, OMP_TRAIT_SET_DEVICE,
- OMP_TRAIT_DEVICE_KIND);
- bool has_arch = omp_get_context_selector (ctx, OMP_TRAIT_SET_DEVICE,
- OMP_TRAIT_DEVICE_ARCH);
- bool has_isa = omp_get_context_selector (ctx, OMP_TRAIT_SET_DEVICE,
- OMP_TRAIT_DEVICE_ISA);
+ bool has_kind
+ = (omp_get_context_selector (ctx, OMP_TRAIT_SET_DEVICE,
+ OMP_TRAIT_DEVICE_KIND)
+ || omp_get_context_selector (ctx, OMP_TRAIT_SET_TARGET_DEVICE,
+ OMP_TRAIT_DEVICE_KIND));
I do not completely understand how that's used, but I have the feeling
that having a match that contains both 'device' and 'target_device' is
not correctly handled here.
int *scores
= (int *) alloca ((2 * nconstructs + 2) * sizeof (int));
That's not new but I have the feeling it should be '+ 3' and not '+ 2'
for device or target_device + and having both device and target device,
it might even need to be + 6.
I also wonder whether 'alloca' will really work - or only when inlined
at all call sites. (Currenlty, it is called 6 times - and a 7th time
with this patch.)
That's how far I came and before having to leave for now. (And having no
time to think about what I wrote about 'score'.)
Tobias
