diff mbox series

[OG10] omp-low.c: Avoid offload-target lto1 is-missing error by not-privatizing TREE_READONLY vars

Message ID 4e717a91-6752-1e5e-6af4-89ee01693977@codesourcery.com
State New
Headers show
Series [OG10] omp-low.c: Avoid offload-target lto1 is-missing error by not-privatizing TREE_READONLY vars | expand

Commit Message

Tobias Burnus July 16, 2020, 1:53 p.m. UTC 
[This is an OpenACC issue but I would not completely surprised if
something similar could occur for OpenMP offloading as well.
However, the patch is for an OpenACC-specific function.]

This issue occurs for libgomp.oacc-fortran/privatized-ref-2.f90, for which
on the device lto1 complains:
lto1: fatal error: /tmp/ccEGJTZN.o: section A.13.1.21 is missing Here,
"A.13" is a TREE_STATIC, TREE_READONLY array generated by the Fortran
front-end and containing the array-constructor values, i.e. RHS of:
array = [(-2*i, i = 1, size(array))] That testcase works on the trunk or
on the OG10 (= devel/omp/gcc-10) branch if one reverts the patch "Re-do
OpenACC private variable resolution"
https://gcc.gnu.org/g:2f4b477223fddb84f66e494eb88d1defbd5e04a2 which is
scheduled but not yet submitted for mainline inclusion. The offloading
variable table contains the variable as "A.13.10" (which works fine) and
the problem-causing patch causes that the code   .UNIQUE (OACC_PRIVATE, 0, 0, &parm.9, &A.13);
gets inserted (via the then-added make_oacc_private_marker in omp-low.c).
Here, the decl for 'A.13' does not have a varpool_node entry – and
it is not streamed out as separate entity.
(This IFN_ is then later processed by the target lto1 via omp-offload.c's
execute_oacc_device_lower – where the asm_name "A.13.1" appears.)

[While I do not completely understand why the target LTO does
not contain the symbol, I think the following still makes sense.
(I do understand why the offload var table does not contain it.)]


If the variable is TREE_READONLY, there is no need to pass it
through the variable-privatization bits.

The current check is for VAR_P and TREE_ADDRESSABLE. For the fix,
one could use:
   !TREE_READONLY
or
   !(TREE_READONLY && TREE_STATIC)
or
   !(TREE_READONLY && (TREE_STATIC || DECL_EXTERNAL)
I am not sure what makes more sense. I initially used the first
version and then moved to the last. Thoughts?

Additional comments?
Does it look OK for OG10?

Tobias

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter

Comments

Julian Brown July 23, 2020, 4:10 p.m. UTC | #1 
On Thu, 16 Jul 2020 15:53:54 +0200
Tobias Burnus <tobias@codesourcery.com> wrote:

> [This is an OpenACC issue but I would not completely surprised if
> something similar could occur for OpenMP offloading as well.
> However, the patch is for an OpenACC-specific function.]
> 
> This issue occurs for libgomp.oacc-fortran/privatized-ref-2.f90, for
> which on the device lto1 complains:
> lto1: fatal error: /tmp/ccEGJTZN.o: section A.13.1.21 is missing
> Here, "A.13" is a TREE_STATIC, TREE_READONLY array generated by the
> Fortran front-end and containing the array-constructor values, i.e.
> RHS of: array = [(-2*i, i = 1, size(array))] That testcase works on
> the trunk or on the OG10 (= devel/omp/gcc-10) branch if one reverts
> the patch "Re-do OpenACC private variable resolution" 
> https://gcc.gnu.org/g:2f4b477223fddb84f66e494eb88d1defbd5e04a2 which
> is scheduled but not yet submitted for mainline inclusion. The
> offloading variable table contains the variable as "A.13.10" (which
> works fine) and the problem-causing patch causes that the code
> .UNIQUE (OACC_PRIVATE, 0, 0, &parm.9, &A.13); gets inserted (via the
> then-added make_oacc_private_marker in omp-low.c). Here, the decl for
> 'A.13' does not have a varpool_node entry – and it is not streamed
> out as separate entity. (This IFN_ is then later processed by the
> target lto1 via omp-offload.c's execute_oacc_device_lower – where the
> asm_name "A.13.1" appears.)
> 
> [While I do not completely understand why the target LTO does
> not contain the symbol, I think the following still makes sense.
> (I do understand why the offload var table does not contain it.)]
> 
> 
> If the variable is TREE_READONLY, there is no need to pass it
> through the variable-privatization bits.
> 
> The current check is for VAR_P and TREE_ADDRESSABLE. For the fix,
> one could use:
>    !TREE_READONLY
> or
>    !(TREE_READONLY && TREE_STATIC)
> or
>    !(TREE_READONLY && (TREE_STATIC || DECL_EXTERNAL)
> I am not sure what makes more sense. I initially used the first
> version and then moved to the last. Thoughts?

I don't know when we'd have TREE_READONLY and DECL_EXTERNAL in a
situation where it made sense to mark the decl private using this
mechanism -- but I'll defer to your judgement!

> Additional comments?
> Does it look OK for OG10?

Looks OK to me, FWIW.

Thanks,

Julian
Tobias Burnus July 27, 2020, 6:40 a.m. UTC | #2 
Now further comments – and for a vendor branch; hence:
Now installed on OG10 = devel/omp/gcc-10 in
commit https://gcc.gnu.org/g:9eb0117bd6c51290ce4d0f1313c0f368185a699d

Cheers,

Tobias

On 7/23/20 6:10 PM, Julian Brown wrote:
> On Thu, 16 Jul 2020 15:53:54 +0200
> Tobias Burnus <tobias@codesourcery.com> wrote:
> …
>> Additional comments?
>> Does it look OK for OG10?
> Looks OK to me, FWIW.
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
diff mbox series

Patch

omp-low.c: Avoid offload-target lto1 is-missing error by not-privatizing TREE_READONLY vars

gcc/ChangeLog:

	* omp-low.c (oacc_record_private_var_clauses,
	oacc_record_vars_in_bind): Do not privatize read-only static/exernal
	variables.
diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index 09603635350..bd3866c4d2e 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -9813,7 +9813,9 @@  oacc_record_private_var_clauses (omp_context *ctx, tree clauses)
     if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_PRIVATE)
       {
 	tree decl = OMP_CLAUSE_DECL (c);
-	if (VAR_P (decl) && TREE_ADDRESSABLE (decl))
+	if (VAR_P (decl) && TREE_ADDRESSABLE (decl)
+	    && !(TREE_READONLY (decl)
+		 && (TREE_STATIC (decl) || DECL_EXTERNAL (decl))))
 	  ctx->oacc_addressable_var_decls->safe_push (decl);
       }
 }
@@ -9828,7 +9830,8 @@  oacc_record_vars_in_bind (omp_context *ctx, tree bindvars)
     return;
 
   for (tree v = bindvars; v; v = DECL_CHAIN (v))
-    if (VAR_P (v) && TREE_ADDRESSABLE (v))
+    if (VAR_P (v) && TREE_ADDRESSABLE (v)
+	&& !(TREE_READONLY (v) && (TREE_STATIC (v) || DECL_EXTERNAL (v))))
       ctx->oacc_addressable_var_decls->safe_push (v);
 }