Hi!
My recent C FE lval folding improvements apparently broke OpenMP/OpenACC
handling of constant size VLAs, e.g.
const int size = 4096;
int array[size];
The problem is that the OpenMP/ACC gimplification/lowering/expansion relies
on TREE_CODE (DECL_SIZE_UNIT ()) != INTEGER_CST, or !TREE_CONSTANT
(TYPE_SIZE_UNIT ()) or variably_modified_type_p predicates to find VLAs that
have been processed by gimplify_vla_decl and need special handling.
The problem is that as the C FE got better at folding stuff, we end up with
decls that have non-INTEGER_CST DECL_SIZE_UNIT which after
gimplify_one_sizepos becomes INTEGER_CST, so the only way to distinguish it
from non-vlas is that it has certain DECL_VALUE_EXPR. There are many other
reasons why something could have a DECL_VALUE_EXPR, so checking that sounds
very error-prone.
This patch instead treats such arrays as non-VLAs, by first gimplifying
DECL_SIZE_UNIT and only then checking if it is non-INTEGER_CST.
It will surely result in better code, even when not using OpenMP/OpenACC,
some people think const in C is the same thing as C++, on the other side,
there might be some carefully crafted code that uses these weirdo constant
VLAs and expect them to be deallocated every time.
{ const int n = 4096*4096; int vla[n]; foo (vla); }
{ const int n = 4096*4096; float vla[n]; bar (vla); }
...
even when for some reason we wouldn't be able to share the constant size
array stack memory.
Is this acceptable optimization anyway? Bootstrapped/regtested on
x86_64-linux and i686-linux.
The alternative would be to add some bit on the decls that would tell the
rest of gimplifier and middle-end that gimplify_vla_decl has been done on it
and it needs to be treated specially.
2018-01-11 Jakub Jelinek <ja...@redhat.com>
PR libgomp/83590
* gimplify.c (gimplify_vla_decl): Don't call gimplify_one_sizepos
for DEC_SIZE and DECL_SIZE_UNIT here...
(gimplify_decl_expr): ... but here before testing if DECL_SIZE_UNIT
is INTEGER_CST.
(gimplify_target_expr): And here too. Call gimplify_type_sizes
always, not just for VLAs. Check DECL_SIZE_UNIT rather than DECL_SIZE
for consistency with gimplify_decl_expr.
* gcc.target/i386/stack-check-18.c (main): Drop const from size
variable.
--- gcc/gimplify.c.jj 2018-01-03 10:19:55.887534074 +0100
+++ gcc/gimplify.c 2018-01-11 10:10:37.733519519 +0100
@@ -1587,9 +1587,6 @@ gimplify_vla_decl (tree decl, gimple_seq
for deferred expansion. */
tree t, addr, ptr_type;
- gimplify_one_sizepos (&DECL_SIZE (decl), seq_p);
- gimplify_one_sizepos (&DECL_SIZE_UNIT (decl), seq_p);
-
/* Don't mess with a DECL_VALUE_EXPR set by the front-end. */
if (DECL_HAS_VALUE_EXPR_P (decl))
return;
@@ -1673,6 +1670,9 @@ gimplify_decl_expr (tree *stmt_p, gimple
tree init = DECL_INITIAL (decl);
bool is_vla = false;
+ gimplify_one_sizepos (&DECL_SIZE (decl), seq_p);
+ gimplify_one_sizepos (&DECL_SIZE_UNIT (decl), seq_p);
+
if (TREE_CODE (DECL_SIZE_UNIT (decl)) != INTEGER_CST
|| (!TREE_STATIC (decl)
&& flag_stack_check == GENERIC_STACK_CHECK
@@ -6548,14 +6548,15 @@ gimplify_target_expr (tree *expr_p, gimp
{
tree cleanup = NULL_TREE;
+ if (!TYPE_SIZES_GIMPLIFIED (TREE_TYPE (temp)))
+ gimplify_type_sizes (TREE_TYPE (temp), pre_p);
+ gimplify_one_sizepos (&DECL_SIZE (temp), pre_p);
+ gimplify_one_sizepos (&DECL_SIZE_UNIT (temp), pre_p);
+
/* TARGET_EXPR temps aren't part of the enclosing block, so add it
to the temps list. Handle also variable length TARGET_EXPRs. */
- if (TREE_CODE (DECL_SIZE (temp)) != INTEGER_CST)
- {
- if (!TYPE_SIZES_GIMPLIFIED (TREE_TYPE (temp)))
- gimplify_type_sizes (TREE_TYPE (temp), pre_p);
- gimplify_vla_decl (temp, pre_p);
- }
+ if (TREE_CODE (DECL_SIZE_UNIT (temp)) != INTEGER_CST)
+ gimplify_vla_decl (temp, pre_p);
else
{
/* Save location where we need to place unpoisoning. It's possible
--- gcc/testsuite/gcc.target/i386/stack-check-18.c.jj 2018-01-03
21:21:38.707907706 +0100
+++ gcc/testsuite/gcc.target/i386/stack-check-18.c 2018-01-11
19:10:11.933703006 +0100
@@ -7,7 +7,7 @@ int f1 (char *);
int
f2 (void)
{
- const int size = 4096;
+ int size = 4096;
char buffer[size];
return f1 (buffer);
}
Jakub
