diff mbox series

[committed] — was: [Patch] OpenMP/C++: Fix declare variant with reference-returning functions

Message ID ee9d86cb-450e-48e3-b081-8aed4ef07b78@baylibre.com
State New
Headers
Series [committed] — was: [Patch] OpenMP/C++: Fix declare variant with reference-returning functions |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 fail Patch failed to apply

Commit Message

Tobias Burnus Oct. 31, 2024, 10:37 a.m. UTC 
  Tobias Burnus wrote:
> Before the patch, the included testcase fails with:
>
> declare-variant-9.C:4:29: error: could not find variant declaration
>      4 | #pragma omp declare variant(variant_fn) match(user={condition(1)})
>        |                             ^~~~~~~~~~
>
> Comments, remarks, suggestions before I commit it?

I realized that a template testcase was missing – hence, I extended
the testcase; cf. attachment.

→ Committed as r15-4799-gf011f8908182fd

BTW: Initially, the template testcase did not work; it turned out that I made
a typo (tmpl vs templ) in the variant name, which GCC did/does not diagnose.

I discussed with Jakub, who will take care of adding a diagnostic. Thanks!

Tobias

Comments

Jakub Jelinek Oct. 31, 2024, 10:46 a.m. UTC | #1 
On Thu, Oct 31, 2024 at 11:37:35AM +0100, Tobias Burnus wrote:
> Tobias Burnus wrote:
> > Before the patch, the included testcase fails with:
> > 
> > declare-variant-9.C:4:29: error: could not find variant declaration
> >      4 | #pragma omp declare variant(variant_fn) match(user={condition(1)})
> >        |                             ^~~~~~~~~~
> > 
> > Comments, remarks, suggestions before I commit it?
> 
> I realized that a template testcase was missing – hence, I extended
> the testcase; cf. attachment.
> 
> → Committed as r15-4799-gf011f8908182fd
> 
> BTW: Initially, the template testcase did not work; it turned out that I made
> a typo (tmpl vs templ) in the variant name, which GCC did/does not diagnose.
> 
> I discussed with Jakub, who will take care of adding a diagnostic. Thanks!
> 
> Tobias

> commit f011f8908182fd05ddd9a34881507b8584c44fb2
> Author: Tobias Burnus <tburnus@baylibre.com>
> Date:   Thu Oct 31 11:28:57 2024 +0100
> 
>     OpenMP/C++: Fix declare variant with reference-returning functions
>     
>     gcc/cp/ChangeLog:
>     
>             * decl.cc (omp_declare_variant_finalize_one): Strip indirect ref
>             around variant-function call when processing a variant.
>     
>     gcc/testsuite/ChangeLog:
>     
>             * g++.dg/gomp/declare-variant-9.C: New test.
> 
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 0bc320a2b39..b638f3af294 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -8375,6 +8375,9 @@ omp_declare_variant_finalize_one (tree decl, tree attr)
>    if (variant == error_mark_node && !processing_template_decl)
>      return true;
>  
> +  if (TREE_CODE (variant) == INDIRECT_REF)
> +    variant = TREE_OPERAND (variant, 0);
> +
>    variant = cp_get_callee_fndecl_nofold (variant);

I think this should be just
  variant = cp_get_callee_fndecl_nofold (STRIP_REFERENCE_REF (variant));
instead.

	Jakub
Tobias Burnus Oct. 31, 2024, 11:42 a.m. UTC | #2 
Hi Jakub,

Jakub Jelinek wrote:
>> +  if (TREE_CODE (variant) == INDIRECT_REF)
>> +    variant = TREE_OPERAND (variant, 0);
>> +
>>     variant = cp_get_callee_fndecl_nofold (variant);
> I think this should be just
>    variant = cp_get_callee_fndecl_nofold (STRIP_REFERENCE_REF (variant));
> instead.

… which clearly shows that I only know the C++ FE superficially.

Yes, while at the end identical, that reads much nicer!
Thanks for the suggestion → commit r15-4801-gf7ae087ef0132b

Tobias
diff mbox series

Patch

commit f011f8908182fd05ddd9a34881507b8584c44fb2
Author: Tobias Burnus <tburnus@baylibre.com>
Date:   Thu Oct 31 11:28:57 2024 +0100

    OpenMP/C++: Fix declare variant with reference-returning functions
    
    gcc/cp/ChangeLog:
    
            * decl.cc (omp_declare_variant_finalize_one): Strip indirect ref
            around variant-function call when processing a variant.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/gomp/declare-variant-9.C: New test.

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 0bc320a2b39..b638f3af294 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -8375,6 +8375,9 @@  omp_declare_variant_finalize_one (tree decl, tree attr)
   if (variant == error_mark_node && !processing_template_decl)
     return true;
 
+  if (TREE_CODE (variant) == INDIRECT_REF)
+    variant = TREE_OPERAND (variant, 0);
+
   variant = cp_get_callee_fndecl_nofold (variant);
   input_location = save_loc;
 
diff --git a/gcc/testsuite/g++.dg/gomp/declare-variant-9.C b/gcc/testsuite/g++.dg/gomp/declare-variant-9.C
new file mode 100644
index 00000000000..7e26d8b11ae
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gomp/declare-variant-9.C
@@ -0,0 +1,29 @@ 
+/* { dg-additional-options "-fdump-tree-gimple" } */
+int &variant_fn();
+
+#pragma omp declare variant(variant_fn) match(user={condition(1)})
+int &bar();
+
+void sub(int &a)
+{
+  bar();
+  a = bar(); 
+}
+
+template<typename T>
+T &templ_var_fn(T x);
+
+#pragma omp declare variant(templ_var_fn) match(user={condition(1)})
+template<typename T>
+T &templ_base_fn(T x);
+
+void run(int &b)
+{
+  templ_base_fn<int>(5);
+  b = templ_base_fn<int>(7); 
+}
+
+/* { dg-final { scan-tree-dump "  variant_fn \\(\\);" "gimple" } } */
+/* { dg-final { scan-tree-dump "  _1 = variant_fn \\(\\);" "gimple" } } */
+/* { dg-final { scan-tree-dump "  templ_var_fn<int> \\(5\\);" "gimple" } } */
+/* { dg-final { scan-tree-dump "  _1 = templ_var_fn<int> \\(7\\);" "gimple" } } */