From patchwork Thu Aug 25 13:36:19 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 662790 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org 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 3sKlZw67Wtz9sBc for ; Thu, 25 Aug 2016 23:36:55 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=pq8MjIqf; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=oaDut32jCTcxqh11B q0JQA8yHfoqCTEQA9ns8QTCY8YWLrganAuw7hoz0kcWSWBHp/tN257OQzyQxjgDT uw5h0iHHFkh6a16s0FdsnR92129SjLPJ04X+UgDR73gUrYjZuvuKCP70TbXxWIeY Cejndmljqeh6iPSnOk/KnoNLt0= 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 :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=ShPfTOzI/2mZWK7KzA8grq4 XekM=; b=pq8MjIqfXq0/nGzaZJFwI9f0Y5wBEQR4kqdblRVmJQJhMoZyPh4bjve H/4q5xRpREbk9bVWHVlVVMvRa5Yu00TtOY861njnKCsuM3VK2EwnABvizUG0BW2+ yGbEvYb7VJkh6JeV8J2JpTj0A0xDmSnUr28WYYPMglgeuN5yu61Y= Received: (qmail 100572 invoked by alias); 25 Aug 2016 13:36:46 -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 100541 invoked by uid 89); 25 Aug 2016 13:36:43 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 25 Aug 2016 13:36:32 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-01.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1bcupZ-0002Sl-GJ from Tom_deVries@mentor.com ; Thu, 25 Aug 2016 06:36:30 -0700 Received: from [127.0.0.1] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.3.224.2; Thu, 25 Aug 2016 14:36:27 +0100 Subject: Re: [PATCH, PR70955] Tag {ms, sysv}_va_list_type_node with {ms, sysv}_abi attribute To: Richard Biener References: <0b95bce7-65c8-b122-9125-9e7a19041df3@mentor.com> CC: GCC Patches From: Tom de Vries Message-ID: <774c26d6-f93a-c816-be78-4371c8af4708@mentor.com> Date: Thu, 25 Aug 2016 15:36:19 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: On 25/08/16 13:48, Richard Biener wrote: > On Wed, 24 Aug 2016, Tom de Vries wrote: > >> Hi, >> >> in PR70955, ix86_canonical_va_list_type fails to recognize a >> __builtin_ms_va_list that was declared in a TU, because its type doesn't have >> the same main variant as the ms_va_list_type_node initialized in lto1. >> >> This patch fixes the PR by tagging ms_va_list_type_node and >> sysv_va_list_type_node with ms_abi/sysv_abi attributes. >> >> sysv_va_list_type_node is of type array of length one with elemtype record, >> and I ran into trouble with both adding the attribute to the array type and >> the record type, so I ended up adding it to the first field type. >> >> Bootstrapped and reg-tested on x86_64. >> >> OK for trunk, 6 branch? > How did you build the sysv_abi tagged struct / array that ended up > not working? My first try to tag the struct was this: ... } ... But we immediately run into: ... : warning: ignoring attributes applied to ‘__va_list_tag’ after definition [-Wattributes] : warning: ignoring attributes applied to ‘struct ’ after definition [-Wattributes] : warning: ignoring attributes applied to ‘struct ’ after definition [-Wattributes] ... I tried to work around that by directly assigning to TYPE_ATTRIBUTES, as implemented in attached patch. But then I run into a libstdc++ build ICE, in mangle.c:write_type: ... tree t = TYPE_MAIN_VARIANT (type); if (TYPE_ATTRIBUTES (t) && !OVERLOAD_TYPE_P (t)) { tree attrs = NULL_TREE; if (tx_safe_fn_type_p (type)) attrs = tree_cons (get_identifier ("transaction_safe"), NULL_TREE, attrs); t = cp_build_type_attribute_variant (t, attrs); } gcc_assert (t != type); ... > I suspect that in parameter passing the array somehow > decays to a pointer-to-element type Yep, as mentioned in more detail at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70955#c23 . That's why I didn't try very hard to get tagging the array with the attribute to work. > so it is important that the > TYPE_MAIN_VARIANT of the record type already contains the attribute. AFAIU, the record type is its own TYPE_MAIN_VARIANT, so it contains the attributes. Thanks, - Tom diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 2639c8c..54a0ef9 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -10548,6 +10548,8 @@ ix86_build_builtin_va_list_64 (void) layout_type (record); + TYPE_ATTRIBUTES (record) = tree_cons (get_identifier ("sysv_abi"), NULL_TREE, NULL_TREE); + /* The correct type is an array type of one element. */ return build_array_type (record, build_index_type (size_zero_node)); } @@ -10561,16 +10563,16 @@ ix86_build_builtin_va_list (void) if (TARGET_64BIT) { /* Initialize ABI specific va_list builtin types. */ - tree sysv_va_list, ms_va_list; - - sysv_va_list = ix86_build_builtin_va_list_64 (); - sysv_va_list_type_node = build_variant_type_copy (sysv_va_list); + sysv_va_list_type_node = ix86_build_builtin_va_list_64 (); /* For MS_ABI we use plain pointer to argument area. */ - ms_va_list = build_pointer_type (char_type_node); - ms_va_list_type_node = build_variant_type_copy (ms_va_list); + tree char_ptr_type = build_pointer_type (char_type_node); + tree attr = tree_cons (get_identifier ("ms_abi"), NULL_TREE, NULL_TREE); + ms_va_list_type_node = build_type_attribute_variant (char_ptr_type, attr); - return (ix86_abi == MS_ABI) ? ms_va_list : sysv_va_list; + return ((ix86_abi == MS_ABI) + ? ms_va_list_type_node + : sysv_va_list_type_node); } else { @@ -48563,8 +48565,6 @@ ix86_fn_abi_va_list (tree fndecl) static tree ix86_canonical_va_list_type (tree type) { - tree wtype, htype; - /* Resolve references and pointers to va_list type. */ if (TREE_CODE (type) == MEM_REF) type = TREE_TYPE (type); @@ -48573,64 +48573,24 @@ ix86_canonical_va_list_type (tree type) else if (POINTER_TYPE_P (type) && TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE) type = TREE_TYPE (type); - if (TARGET_64BIT && va_list_type_node != NULL_TREE) + if (TARGET_64BIT) { - wtype = va_list_type_node; - gcc_assert (wtype != NULL_TREE); - htype = type; - if (TREE_CODE (wtype) == ARRAY_TYPE) - { - /* If va_list is an array type, the argument may have decayed - to a pointer type, e.g. by being passed to another function. - In that case, unwrap both types so that we can compare the - underlying records. */ - if (TREE_CODE (htype) == ARRAY_TYPE - || POINTER_TYPE_P (htype)) - { - wtype = TREE_TYPE (wtype); - htype = TREE_TYPE (htype); - } - } - if (TYPE_MAIN_VARIANT (wtype) == TYPE_MAIN_VARIANT (htype)) - return va_list_type_node; - wtype = sysv_va_list_type_node; - gcc_assert (wtype != NULL_TREE); - htype = type; - if (TREE_CODE (wtype) == ARRAY_TYPE) - { - /* If va_list is an array type, the argument may have decayed - to a pointer type, e.g. by being passed to another function. - In that case, unwrap both types so that we can compare the - underlying records. */ - if (TREE_CODE (htype) == ARRAY_TYPE - || POINTER_TYPE_P (htype)) - { - wtype = TREE_TYPE (wtype); - htype = TREE_TYPE (htype); - } - } - if (TYPE_MAIN_VARIANT (wtype) == TYPE_MAIN_VARIANT (htype)) - return sysv_va_list_type_node; - wtype = ms_va_list_type_node; - gcc_assert (wtype != NULL_TREE); - htype = type; - if (TREE_CODE (wtype) == ARRAY_TYPE) + if (lookup_attribute ("ms_abi", TYPE_ATTRIBUTES (type))) + return ms_va_list_type_node; + + if ((TREE_CODE (type) == ARRAY_TYPE + && integer_zerop (array_type_nelts (type))) + || POINTER_TYPE_P (type)) { - /* If va_list is an array type, the argument may have decayed - to a pointer type, e.g. by being passed to another function. - In that case, unwrap both types so that we can compare the - underlying records. */ - if (TREE_CODE (htype) == ARRAY_TYPE - || POINTER_TYPE_P (htype)) - { - wtype = TREE_TYPE (wtype); - htype = TREE_TYPE (htype); - } + tree elem_type = TREE_TYPE (type); + if (TREE_CODE (elem_type) == RECORD_TYPE + && lookup_attribute ("sysv_abi", TYPE_ATTRIBUTES (elem_type))) + return sysv_va_list_type_node; } - if (TYPE_MAIN_VARIANT (wtype) == TYPE_MAIN_VARIANT (htype)) - return ms_va_list_type_node; + return NULL_TREE; } + return std_canonical_va_list_type (type); }