Re: [PATCH v4 4/7] OpenMP: C++ front-end support for dispatch + adjust_args

[Tobias Burnus]

Hi PA,

Regarding the update middle end patch, included in this patch, i.e.
v4.3-0002-OpenMP-middle-end-support-for-dispatch-adjust_arg.patch:

@@ -4071,23 +4073,136 @@ gimplify_call_expr (tree *expr_p, gimple_seq *pre_p, 
bool want_value)

[...]

+             if (flag_openmp && gimplify_omp_ctxp != NULL
+                 && gimplify_omp_ctxp->code == OMP_DISPATCH
+                 && !gimplify_omp_ctxp->in_call_args && !integer_zerop (*arg_p)
+                 && EXPR_P (CALL_EXPR_FN (*expr_p))
+                 && DECL_P (TREE_OPERAND (CALL_EXPR_FN (*expr_p), 0))
Please add a line break before '&& !integer_zerop (...)'.
It is very easy to miss that part, given that the lines are very long,
such that keeps just parsing the '&&'.

Otherwise, this part LGTM (assuming that you have a testcase for it).

* * *

Regarding C++:


Paul-Antoine Arras wrote:

In C++, there is no equivalent to 
c_parser_postfix_expression_after_primary. So I am now calling 
cp_parser_postfix_expression, which parses not only the argument list 
but the whole function call.
… and …
int variant_fn(int *, int * = nullptr);
(This part is now handled via the middle end patch, cf. above.)
int& variant_fn();
#pragma omp declare variant(variant_fn) match(construct={dispatch})
int& bar();

void sub(int a)
{
   #pragma omp dispatch
     bar();
   #pragma omp dispatch
     a = bar();

The issue that 'variant_fn' is not found is now fixed →r15-4799-gf011f8908182fd However, the code now stumbles over the 
indirect ref, such that two (?) STRIP_REFERENCE_REF are missing. Can you 
look into this? — See also the next comment. * * *

+cp_parser_omp_dispatch_body (cp_parser *parser)
+{
...
+  if (!(TREE_CODE (rhs) == CALL_EXPR
+       || (TREE_CODE (rhs) == INDIRECT_REF
+           && TREE_CODE (TREE_OPERAND (rhs, 0)) == CALL_EXPR)))
+    {

I think the last == should be !=, otherwise the code does not make sense.

Additionally, I think it is cleaner to write:

if (!TREE_CODE (STRIP_REFERENCE_REF (rhs)) == CALL_EXPR)

(Thanks to Jakub for pointing me to that macro for my patch.)

* * *

I still need to continue looking through the patch, but I have a second
testcase that fails - this time with an ICE:

        ../../../repos/gcc/gcc/toplev.cc:323
0xd85322 cp_build_modify_expr(unsigned int, tree_node*, tree_code, tree_node*, 
int)
        ../../../repos/gcc/gcc/cp/typeck.cc:9743
0xc5bfae cp_parser_omp_dispatch_body
        ../../../repos/gcc/gcc/cp/parser.cc:49237

Tobias
template<typename T>
T templ_var_fn(T x);

#pragma omp declare variant(templ_var_fn) match(construct={dispatch}) adjust_args(need_device_ptr: x)
template<typename T>
T templ_base_fn(T x);

template<typename T, typename TB>
void f(int *y, TB x)
{
#pragma omp dispatch nocontext (x)
  y = templ_base_fn<T> (y); 
}

void bar()
{
  int a;
  bool b = true;
  f<int*,bool> (&a, b);
}