Only apply adjust_args in OpenMP dispatch if variant substitution occurs
Checks
Commit Message
This is a followup to
084ea8ad584 OpenMP: middle-end support for dispatch + adjust_args.
This patch fixes a bug that caused arguments in an OpenMP dispatch call to be
modified even when no variant substitution occurred.
gcc/ChangeLog:
* gimplify.cc (gimplify_call_expr): Create variable
variant_substituted_p to control whether adjust_args applies.
---
gcc/gimplify.cc | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
Comments
Apologies, I forgot to add the testcase. Please find attached an updated
patch.
On 06/01/2025 17:12, Paul-Antoine Arras wrote:
> This is a followup to
> 084ea8ad584 OpenMP: middle-end support for dispatch + adjust_args.
>
> This patch fixes a bug that caused arguments in an OpenMP dispatch call to be
> modified even when no variant substitution occurred.
>
> gcc/ChangeLog:
>
> * gimplify.cc (gimplify_call_expr): Create variable
> variant_substituted_p to control whether adjust_args applies.
> ---
> gcc/gimplify.cc | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
> index bd324be926a..251d581f44c 100644
> --- a/gcc/gimplify.cc
> +++ b/gcc/gimplify.cc
> @@ -3857,7 +3857,8 @@ gimplify_call_expr (tree *expr_p, gimple_seq *pre_p, bool want_value)
> enum gimplify_status ret;
> int i, nargs;
> gcall *call;
> - bool builtin_va_start_p = false, omp_dispatch_p = false;
> + bool builtin_va_start_p = false, omp_dispatch_p = false,
> + variant_substituted_p = false;
> location_t loc = EXPR_LOCATION (*expr_p);
>
> gcc_assert (TREE_CODE (*expr_p) == CALL_EXPR);
> @@ -4035,7 +4036,10 @@ gimplify_call_expr (tree *expr_p, gimple_seq *pre_p, bool want_value)
> {
> tree variant = omp_resolve_declare_variant (fndecl);
> if (variant != fndecl)
> - CALL_EXPR_FN (*expr_p) = build1 (ADDR_EXPR, fnptrtype, variant);
> + {
> + CALL_EXPR_FN (*expr_p) = build1 (ADDR_EXPR, fnptrtype, variant);
> + variant_substituted_p = true;
> + }
> }
>
> /* There is a sequence point before the call, so any side effects in
> @@ -4325,8 +4329,9 @@ gimplify_call_expr (tree *expr_p, gimple_seq *pre_p, bool want_value)
> }
> }
>
> - if ((need_device_ptr && !is_device_ptr)
> - || (need_device_addr && !has_device_addr))
> + if (variant_substituted_p
> + && ((need_device_ptr && !is_device_ptr)
> + || (need_device_addr && !has_device_addr)))
> {
> if (dispatch_device_num == NULL_TREE)
> {
Hi PA,
Paul-Antoine Arras wrote:
> On 06/01/2025 17:12, Paul-Antoine Arras wrote:
>> This is a followup to
>> 084ea8ad584 OpenMP: middle-end support for dispatch + adjust_args.
>>
>> This patch fixes a bug that caused arguments in an OpenMP dispatch
>> call to be
>> modified even when no variant substitution occurred.
>>
LGTM. Thanks!
Tobias
@@ -3857,7 +3857,8 @@ gimplify_call_expr (tree *expr_p, gimple_seq *pre_p, bool want_value)
enum gimplify_status ret;
int i, nargs;
gcall *call;
- bool builtin_va_start_p = false, omp_dispatch_p = false;
+ bool builtin_va_start_p = false, omp_dispatch_p = false,
+ variant_substituted_p = false;
location_t loc = EXPR_LOCATION (*expr_p);
gcc_assert (TREE_CODE (*expr_p) == CALL_EXPR);
@@ -4035,7 +4036,10 @@ gimplify_call_expr (tree *expr_p, gimple_seq *pre_p, bool want_value)
{
tree variant = omp_resolve_declare_variant (fndecl);
if (variant != fndecl)
- CALL_EXPR_FN (*expr_p) = build1 (ADDR_EXPR, fnptrtype, variant);
+ {
+ CALL_EXPR_FN (*expr_p) = build1 (ADDR_EXPR, fnptrtype, variant);
+ variant_substituted_p = true;
+ }
}
/* There is a sequence point before the call, so any side effects in
@@ -4325,8 +4329,9 @@ gimplify_call_expr (tree *expr_p, gimple_seq *pre_p, bool want_value)
}
}
- if ((need_device_ptr && !is_device_ptr)
- || (need_device_addr && !has_device_addr))
+ if (variant_substituted_p
+ && ((need_device_ptr && !is_device_ptr)
+ || (need_device_addr && !has_device_addr)))
{
if (dispatch_device_num == NULL_TREE)
{