From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 33226 invoked by alias); 28 Aug 2015 02:08:10 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 33081 invoked by uid 89); 28 Aug 2015 02:08:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 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 (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 28 Aug 2015 02:08:08 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id F1D78A8F; Fri, 28 Aug 2015 02:08:06 +0000 (UTC) Received: from [10.10.116.36] (ovpn-116-36.rdu2.redhat.com [10.10.116.36]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t7S2831I004373; Thu, 27 Aug 2015 22:08:05 -0400 Subject: Re: C++ delayed folding branch review To: Kai Tietz References: <557A5214.7060106@redhat.com> <55B911DD.30105@redhat.com> <55BA5667.9040200@redhat.com> <55BAACF9.7040707@redhat.com> <597173047.4338388.1438379666336.JavaMail.zimbra@redhat.com> <55BEE4CE.9070706@redhat.com> <55BF8B2B.9040001@redhat.com> <55DE7C55.6030207@redhat.com> <55DF1042.9020603@redhat.com> Cc: Kai Tietz , gcc-patches List From: Jason Merrill Message-ID: <55DFC283.8040409@redhat.com> Date: Fri, 28 Aug 2015 02:12:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2015-08/txt/msg01759.txt.bz2 On 08/27/2015 09:38 AM, Kai Tietz wrote: > 2015-08-27 15:27 GMT+02:00 Jason Merrill : >> On 08/27/2015 06:39 AM, Kai Tietz wrote: >>> >>> 2015-08-27 4:56 GMT+02:00 Jason Merrill : >>>> >>>> On 08/24/2015 03:15 AM, Kai Tietz wrote: >>>>> >>>>> 2015-08-03 17:39 GMT+02:00 Jason Merrill : >>>>>> >>>>>> On 08/03/2015 05:42 AM, Kai Tietz wrote: >>>>>>> >>>>>>> 2015-08-03 5:49 GMT+02:00 Jason Merrill : >>>>>>>> >>>>>>>> On 07/31/2015 05:54 PM, Kai Tietz wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> The "STRIP_NOPS-requirement in 'reduced_constant_expression_p'" I >>>>>>>>> could >>>>>>>>> remove, but for one case in constexpr. Without folding we don't do >>>>>>>>> type-sinking/raising. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Right. >>>>>>>> >>>>>>>>> So binary/unary operations might be containing cast, which were in >>>>>>>>> the >>>>>>>>> past unexpected. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Why aren't the casts folded away? >>>>>>> >>>>>>> >>>>>>> >>>>>>> On such cast constructs, as for this vector-sample, we can't fold away >>>>>> >>>>>> >>>>>> >>>>>> Which testcase is this? >>>>> >>>>> >>>>> >>>>> It is the g++.dg/ext/vector20.C testcase. IIRC I mentioned this >>>>> testcase already earlier as reference, but I might be wrong here. >>>> >>>> >>>> >>>> I don't see any casts in that testcase. So the compiler is introducing >>>> introducing conversions back and forth between const and non-const, then? >>>> I >>>> suppose it doesn't so much matter where they come from, they should be >>>> folded away regardless. >>> >>> >>> The cast gets introduced in convert.c about line 836 in function >>> convert_to_integer_1 AFAIK. There should be the alternative solution >>> for this issue by disallowing for PLUS/MINUS/... expressions the >>> sinking of the cast into the expression, if dofold is false, and type >>> has same width as inner_type, and is of vector-kind. >> >> >> Why would we be calling convert_to_integer for conversions between vector >> types? >> >>>>>>> the cast chain. The difference here to none-delayed-folding branch is >>>>>>> that the cast isn't moved out of the plus-expr. What we see now is >>>>>>> (plus ((vec) (const vector ...) { .... }), ...). Before we had (vec) >>>>>>> (plus (const vector ...) { ... }). >>>>>> >>>>>> >>>>>> >>>>>> How could a PLUS_EXPR be considered a reduced constant, regardless of >>>>>> where >>>>>> the cast is? >>>>> >>>>> >>>>> >>>>> Of course it is just possible to sink out a cast from PLUS_EXPR, in >>>>> pretty few circumstance (eg. on constants if both types just differ in >>>>> const-attribute, if conversion is no view-convert). >>>> >>>> >>>> >>>> I don't understand how this is an answer to my question. >>> >>> >>> (vec) (const vector) { ... } expression can't be folded. >> >> >> It currently isn't folded, but why can't we change that? >> >>> This cast to >>> none-const variant happens due the 'constexpr v = v + >>> ' pattern in testcase. v is still of type vec, even >>> if function itself is constexpr. >> >> >> I don't see that pattern in the testcase: >> >> typedef long vec __attribute__((vector_size (2 * sizeof (long)))); >> constexpr vec v = { 3, 4 }; >> constexpr vec s = v + v; >> constexpr vec w = __builtin_shuffle (v, v); >> >> If we have v + constant-value, that's because we pulled out the constant >> value of one of the v's, which we ought to be doing for both of them. >> >>>>>>>>> On verify_constant we check by reduced_constant_expression_p, if >>>>>>>>> value >>>>>>>>> is >>>>>>>>> a constant. We don't handle here, that NOP_EXPRs are something we >>>>>>>>> want to >>>>>>>>> look through here, as it doesn't change anything if this is a >>>>>>>>> constant, or >>>>>>>>> not. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> NOPs around constants should have been folded away by the time we get >>>>>>>> there. >>>>>>> >>>>>>> >>>>>>> >>>>>>> Not in this cases, as the we actually have here a switch from const to >>>>>>> none-const. So there is an attribute-change, which we can't ignore in >>>>>>> general. >>>>>> >>>>>> >>>>>> >>>>>> I wasn't suggesting we ignore it, we should be able to change the type >>>>>> of >>>>>> the vector_cst. >>>>> >>>>> >>>>> >>>>> Well, the vector_cst we can change type, but this wouldn't help >>>>> AFAICS. As there is still one cast surviving within PLUS_EXPR for the >>>>> other operand. >>>> >>>> >>>> >>>> Isn't the other operand also constant? In constexpr evaluation, either >>>> we're dealing with a bunch of constants, in which case we should be >>>> folding >>>> things fully, including conversions between const and non-const, or we >>>> don't >>>> care. >>> >>> >>> No other operand isn't a constant-value. See code-pattern in >>> testcase. It is of type 'vec', which isn't constant (well, 'v' is, >>> but constexpr doesn't know about it). >> >> >> What do you mean, "constexpr doesn't know about it"? >> >>>>> So the way to solve it would be to move such conversion out of the >>>>> expression. For integer-scalars we do this, and for some >>>>> floating-points too. So it might be something we don't handle for >>>>> operations with vector-type. >>>> >>>> >>>> >>>> We don't need to worry about that in constexpr evaluation, since we only >>>> care about constant operands. >>> >>> >>> Sure, but the variable 'v' is the problem, not a constant-value itself. >> >> >>>>>>> But I agree that for constexpr's we could special case cast >>>>>>> from const to none-const (as required in expressions like const vec v >>>>>>> = v + 1). >>>>>> >>>>>> >>>>>> >>>>>> Right. But really this should happen in convert.c, it shouldn't be >>>>>> specific >>>>>> to C++. >>>>> >>>>> >>>>> >>>>> Hmm, maybe. But isn't one of our different goals to move such >>>>> implicit code-modification to match.pd instead? >>>> >>>> >>>> Folding const into a constant is hardly code modification. But perhaps >>>> it >>>> should go into fold_unary_loc:VIEW_CONVERT_EXPR rather than into >>>> convert.c. >>> >>> >>> Hmm, it isn't related to a view-convert. So moving it into >>> fold_unary_loc wouldn't solve here anything. Issue is in constexpr >>> code, not in folding itself. >> >> >> What TREE_CODE does the conversion (vec) (const vector) { ... } use? > > The tree code is a NOP_EXPR. That's probably a bug, seeing as fold_convert and convert_to_vector use VIEW_CONVERT_EXPR. Jason