From patchwork Thu Jan 11 18:24:53 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 859231 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-470862-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="I4vVzm6Q"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3zHZ72663Qz9s7g for ; Fri, 12 Jan 2018 05:25:17 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:reply-to:mime-version :content-type; q=dns; s=default; b=m/t/iTb+itknpqW9GB14/nchTTgxU qVqB4MBrRveEykGWytQp7JmxqfaKa1Ka52JITEIWWOZLB0Dh04SpIAoy0I4tJGVa fY7VeFJ35K+8UY+eZsMbUDxagVah1alx0LE/NBUSKpjwQlHbV6TnI5R0pPYBqau3 iGr+5dTZ5XfzPY= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:reply-to:mime-version :content-type; s=default; bh=ntcPgW44BIQAW79PsLayXhhfW/U=; b=I4v Vzm6Q14A/HdMFPw8hBCvhm3NZM6uiPPaCJjJPcYYfmWTdAww1x4dnhteHeEJZQ1T VfQ5XkOWvff5xXEfatJIOwCdkbkJCncmccbgbkrofcuZt4ob5jXcvtqNsow8gkaS GHPLusggObBwE9g8D1q8nXmcmmPcdncio6/dR8fQ= Received: (qmail 93890 invoked by alias); 11 Jan 2018 18:25:10 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 93874 invoked by uid 89); 11 Jan 2018 18:25:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 11 Jan 2018 18:25:08 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5614A78256; Thu, 11 Jan 2018 18:25:01 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-116-37.ams2.redhat.com [10.36.116.37]) by smtp.corp.redhat.com (Postfix) with ESMTPS id CB954D1F3; Thu, 11 Jan 2018 18:25:00 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id w0BIOtt8008532; Thu, 11 Jan 2018 19:24:56 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id w0BIOrdi008531; Thu, 11 Jan 2018 19:24:53 +0100 Date: Thu, 11 Jan 2018 19:24:53 +0100 From: Jakub Jelinek To: Richard Biener , "Joseph S. Myers" Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] Fix up handling of not really variable VLAs (PR libgomp/83590) Message-ID: <20180111182453.GT1833@tucnak> Reply-To: Jakub Jelinek MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.9.1 (2017-09-22) X-IsSubscribed: yes 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 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. Jakub --- 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); }