| Message ID | bcc85495-c207-be9d-d608-d398e175546c@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
| Series | [LTO] Set proper DECL_ALIGN in offload_handle_link_vars (PR94233) | expand |
On March 20, 2020 10:10:54 PM GMT+01:00, Tobias Burnus <tobias@codesourcery.com> wrote: >When compiling the existing libgomp.c/target-link-1.c with -O3, the >test case was ICEing as DECL_ALIGN was 1 (alias "1U << 0"). > >The reason is that "make_node (VAR_DECL)" calls "SET_DECL_ALIGN (t, 1)" >and unless one overrides this, it stays like that. Here, it later >failed by violating the assert that "align % BITS_PER_UNIT == 0" as >1 % 8 != 0 > >I don't know why this only fails with -O3. (For Nvidia, this >test case is disabled. I don't know why or whether it work(s/ed) >with Intel MIC or HSA.) > >I am not sure what to do about the testcase; it is currently is >run with -O2 – which did not trigger this ICE. I could add >'{ dg-do run }', but this does not make a difference. I could add >'{ dg-additional-options "-O3" }' – if that makes sense. >Better ideas? > >OK for the trunk? It should be TYPE_ALIGN (type). OK with that change. Note this fails to honor target bits so it might be better to lay out the decl properly? >Tobias > >Side notes: > >* The modified code is protected by "#ifdef ACCEL_COMPILER". >* The function was added 2015-12-15; I assume that nothing but >GCN uses it (as Nvidia PTX does not work); hence, I do not see a >reason for backporting. > >* Independent of that patch, it still fails at run time with > "libgomp: Cannot map target variables (size mismatch)" > My feeling is that the following bit flip of omp-offload.c's > add_decls_addresses_to_decl_constructor > is not handled in libgomp/target.c: > if (!is_link_var) > else > isize |= 1ULL << (int_size_in_bytes (const_ptr_type_node) > >----------------- >Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / >Germany >Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, >Alexander Walter
On 3/21/20 8:03 AM, Richard Biener wrote: >> OK for the trunk? > It should be TYPE_ALIGN (type). OK with that change. I am confused. The patch has: + SET_DECL_ALIGN (link_ptr_var, TYPE_ALIGN (ptr_type_node)); which looks correct and to uses already TYPE_ALIGN?!? > Note this fails to honor target bits so it might be better to lay out the decl properly? However, that's a good point. Let's do it properly by calling layout_decl — indirectly, by calling build_decl. OK? Tobias ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
On Sat, 21 Mar 2020, Tobias Burnus wrote: > On 3/21/20 8:03 AM, Richard Biener wrote: > > >> OK for the trunk? > > It should be TYPE_ALIGN (type). OK with that change. > > I am confused. The patch has: > + SET_DECL_ALIGN (link_ptr_var, TYPE_ALIGN (ptr_type_node)); > which looks correct and to uses already TYPE_ALIGN?!? > > > Note this fails to honor target bits so it might be better to lay out the > > decl properly? > > However, that's a good point. Let's do it properly by calling layout_decl > — indirectly, by calling build_decl. > > OK? OK. Thanks, Richard.
Set proper DECL_ALIGN in offload_handle_link_vars (PR94233) gcc/lto/ * lto.c (offload_handle_link_vars): Set DECL_ALIGN. diff --git a/gcc/lto/lto.c b/gcc/lto/lto.c index 39bb5f45c95..93127f7498c 100644 --- a/gcc/lto/lto.c +++ b/gcc/lto/lto.c @@ -566,6 +566,7 @@ offload_handle_link_vars (void) TREE_USED (link_ptr_var) = 1; TREE_STATIC (link_ptr_var) = 1; SET_DECL_MODE (link_ptr_var, TYPE_MODE (type)); + SET_DECL_ALIGN (link_ptr_var, TYPE_ALIGN (ptr_type_node)); DECL_SIZE (link_ptr_var) = TYPE_SIZE (type); DECL_SIZE_UNIT (link_ptr_var) = TYPE_SIZE_UNIT (type); DECL_ARTIFICIAL (link_ptr_var) = 1;
When compiling the existing libgomp.c/target-link-1.c with -O3, the test case was ICEing as DECL_ALIGN was 1 (alias "1U << 0"). The reason is that "make_node (VAR_DECL)" calls "SET_DECL_ALIGN (t, 1)" and unless one overrides this, it stays like that. Here, it later failed by violating the assert that "align % BITS_PER_UNIT == 0" as 1 % 8 != 0 I don't know why this only fails with -O3. (For Nvidia, this test case is disabled. I don't know why or whether it work(s/ed) with Intel MIC or HSA.) I am not sure what to do about the testcase; it is currently is run with -O2 – which did not trigger this ICE. I could add '{ dg-do run }', but this does not make a difference. I could add '{ dg-additional-options "-O3" }' – if that makes sense. Better ideas? OK for the trunk? Tobias Side notes: * The modified code is protected by "#ifdef ACCEL_COMPILER". * The function was added 2015-12-15; I assume that nothing but GCN uses it (as Nvidia PTX does not work); hence, I do not see a reason for backporting. * Independent of that patch, it still fails at run time with "libgomp: Cannot map target variables (size mismatch)" My feeling is that the following bit flip of omp-offload.c's add_decls_addresses_to_decl_constructor is not handled in libgomp/target.c: if (!is_link_var) else isize |= 1ULL << (int_size_in_bytes (const_ptr_type_node) ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter