From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id D1EB03858421 for ; Sat, 8 Jan 2022 06:08:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D1EB03858421 Received: from mail-qv1-f72.google.com (mail-qv1-f72.google.com [209.85.219.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-515-LAgANGngOZW4vB9_cIe5Ew-1; Sat, 08 Jan 2022 01:08:29 -0500 X-MC-Unique: LAgANGngOZW4vB9_cIe5Ew-1 Received: by mail-qv1-f72.google.com with SMTP id jr7-20020a0562142a8700b00411a73d8adaso6856025qvb.3 for ; Fri, 07 Jan 2022 22:08:29 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=vQ634kiM77QskD5XcWC5746ag2DGpBQ03dG0GLRxeHA=; b=B1iws1vZNLJUBfmeNivrFizfdnvlCngp4gXcc9SCRfHCA4IyeJWJlHsa/LLLtKOWLj fpLUiO86HTB+xxGqnIPoqSNI92xY6OcabeyKdDKGppcWpxgAxrEOZVHWm3gtGdJSr4/1 cxOOaw0O+sFyYLLGF5mzjRKZGpe4eJg+ZKzs2QXd5gGZ0T1ljTQy/0tCyXxBJHA082wT zE54TIYrI3ZBM0BUeolfThlFd73krqwDJCyd6P2HedN6jvv+zZnHLY51cy2Y829w8gYj haWkHUcUBTxGSLvgXjcTkcLu3ZrYGAQIpxckVUZo6xdW1wnNwPty0QmTiOCrieA9MZX/ A09w== X-Gm-Message-State: AOAM533DRxbZdfQhOrM9oysK/3JgbLPgRVLPxhq6D5KvGHx9IhOiCENr XyWeCefxn0z0SKqOuONA/CHjmyC6szzCFeIRXxhbNb0ckELvOHmWEUMp8lhQsI1P4/OZKPQu1aB uUL0gnLTfNPK0gTRD1g== X-Received: by 2002:ac8:588b:: with SMTP id t11mr12960123qta.676.1641622108443; Fri, 07 Jan 2022 22:08:28 -0800 (PST) X-Google-Smtp-Source: ABdhPJypZI9rEVuGu3i/1I5PugAsuvyGilLr3RQ5Xhmu0UOW1bSOE3uBM8sHUIbaf5Vt62xvbifj3Q== X-Received: by 2002:ac8:588b:: with SMTP id t11mr12960110qta.676.1641622108028; Fri, 07 Jan 2022 22:08:28 -0800 (PST) Received: from [192.168.1.149] (130-44-159-43.s15913.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id w10sm539566qkp.121.2022.01.07.22.08.26 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 07 Jan 2022 22:08:27 -0800 (PST) Message-ID: <01f0488a-cd4d-bf12-050f-055d1d8fa8ff@redhat.com> Date: Sat, 8 Jan 2022 01:08:25 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.4.1 Subject: Re: [PATCH] c++, match.pd: Evaluate in constant evaluation comparisons like &var1 + 12 == &var2 + 24 [PR89074] To: Jakub Jelinek , Richard Biener Cc: gcc-patches@gcc.gnu.org References: <20220107120639.GK2646553@tucnak> From: Jason Merrill In-Reply-To: <20220107120639.GK2646553@tucnak> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-9.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 08 Jan 2022 06:08:32 -0000 On 1/7/22 07:06, Jakub Jelinek wrote: > Hi! > > The match.pd address_comparison simplification can only handle > ADDR_EXPR comparisons possibly converted to some other type (I wonder > if we shouldn't restrict it in address_compare to casts to pointer > types or pointer-sized integer types, I think we shouldn't optimize > (short) (&var) == (short) (&var2) because we really don't know whether > it will be true or false). On GIMPLE, most of pointer to pointer > casts are useless and optimized away and further we have in > gimple_fold_stmt_to_constant_1 an optimization that folds > &something p+ const_int > into > &MEM_REF[..., off] > On GENERIC, we don't do that and e.g. for constant evaluation it > could be pretty harmful if e.g. such pointers are dereferenced, because > it can lose what exact field it was starting with etc., all it knows > is the base and offset, type and alias set. > Instead of teaching the match.pd address_compare about 3 extra variants > where one or both compared operands are pointer_plus, this patch attempts > to fold operands of comparisons similarly to gimple_fold_stmt_to_constant_1 > before calling fold_binary on it. > There is another thing though, while we do have (x p+ y) p+ z to > x p+ (y + z) simplification which works on GIMPLE well because of the > useless pointer conversions, on GENERIC we can have pointer casts in between > and at that point we can end up with large expressions like > ((type3) (((type2) ((type1) (&var + 2) + 2) + 2) + 2)) > etc. Pointer-plus doesn't really care what exact pointer type it has as > long as it is a pointer, so the following match.pd simplification for > GENERIC only (it is useless for GIMPLE) also moves the cast so that nested > p+ can be simplified. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? LGTM. > Note, I've noticed we don't really diagnose going out of bounds with > pointer_plus (unlike e.g. with ARRAY_REF) during constant evaluation, I > think another patch for cxx_eval_binary_expression with POINTER_PLUS will be > needed. But it isn't clear to me what exactly it should do in case of > subobjects. If we start with address of a whole var, (&var), I guess we > should diagnose if the pointer_plus gets before start of the var (i.e. > "negative") or 1 byte past the end of the var, but what if we start with > &var.field or &var.field[3] ? For &var.field, shall we diagnose out of > bounds of field (except perhaps flexible members? or the whole var? The field. And a flexible member has unknown bounds. > For ARRAY_REFs, I assume we must at least strip all the outer ARRAY_REFs > and so start with &var.field too, right? A strict reading suggests that we should complain about going outside the bounds of the inner array, but flattening multidimensional arrays as you suggest seems reasonable as well. > 2022-01-07 Jakub Jelinek > > PR c++/89074 > gcc/ > * match.pd ((ptr) (x p+ y) p+ z -> (ptr) (x p+ (y + z))): New GENERIC > simplification. > gcc/cp/ > * constexpr.c (cxx_maybe_fold_addr_pointer_plus): New function. > (cxx_eval_binary_expression): Use it. > gcc/testsuite/ > * g++.dg/cpp1y/constexpr-89074-2.C: New test. > * g++.dg/cpp1z/constexpr-89074-1.C: New test. > > --- gcc/match.pd.jj 2022-01-05 20:30:08.768806236 +0100 > +++ gcc/match.pd 2022-01-06 19:59:53.596114417 +0100 > @@ -2143,6 +2143,11 @@ (define_operator_list SYNC_FETCH_AND_AND > (simplify > (pointer_plus (pointer_plus:s @0 @1) @3) > (pointer_plus @0 (plus @1 @3))) > +#if GENERIC > +(simplify > + (pointer_plus (convert:s (pointer_plus:s @0 @1)) @3) > + (convert:type (pointer_plus @0 (plus @1 @3)))) > +#endif > > /* Pattern match > tem1 = (long) ptr1; > --- gcc/cp/constexpr.c.jj 2022-01-03 10:40:48.403063535 +0100 > +++ gcc/cp/constexpr.c 2022-01-06 20:47:44.596623219 +0100 > @@ -3288,6 +3288,38 @@ cxx_fold_pointer_plus_expression (const > return NULL_TREE; > } > > +/* Try to fold expressions like > + (struct S *) (&a[0].D.2378 + 12) > + into > + &MEM [(void *)&a + 12B] > + This is something normally done by gimple_fold_stmt_to_constant_1 > + on GIMPLE, but is undesirable on GENERIC if we are e.g. going to > + dereference the address because some details are lost. > + For pointer comparisons we want such folding though so that > + match.pd address_compare optimization works. */ > + > +static tree > +cxx_maybe_fold_addr_pointer_plus (tree t) > +{ > + while (CONVERT_EXPR_P (t) > + && POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (t, 0)))) > + t = TREE_OPERAND (t, 0); > + if (TREE_CODE (t) != POINTER_PLUS_EXPR) > + return NULL_TREE; > + tree op0 = TREE_OPERAND (t, 0); > + tree op1 = TREE_OPERAND (t, 1); > + if (TREE_CODE (op1) != INTEGER_CST) > + return NULL_TREE; > + while (CONVERT_EXPR_P (op0) > + && POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (op0, 0)))) > + op0 = TREE_OPERAND (op0, 0); > + if (TREE_CODE (op0) != ADDR_EXPR) > + return NULL_TREE; > + op1 = fold_convert (ptr_type_node, op1); > + tree r = fold_build2 (MEM_REF, TREE_TYPE (TREE_TYPE (op0)), op0, op1); > + return build1_loc (EXPR_LOCATION (t), ADDR_EXPR, TREE_TYPE (op0), r); > +} > + > /* Subroutine of cxx_eval_constant_expression. > Like cxx_eval_unary_expression, except for binary expressions. */ > > @@ -3347,6 +3379,15 @@ cxx_eval_binary_expression (const conste > else if (TREE_CODE (rhs) == PTRMEM_CST) > rhs = cplus_expand_constant (rhs); > } > + if (r == NULL_TREE > + && TREE_CODE_CLASS (code) == tcc_comparison > + && POINTER_TYPE_P (TREE_TYPE (lhs))) > + { > + if (tree lhso = cxx_maybe_fold_addr_pointer_plus (lhs)) > + lhs = fold_convert (TREE_TYPE (lhs), lhso); > + if (tree rhso = cxx_maybe_fold_addr_pointer_plus (rhs)) > + rhs = fold_convert (TREE_TYPE (rhs), rhso); > + } > if (code == POINTER_PLUS_EXPR && !*non_constant_p > && integer_zerop (lhs) && !integer_zerop (rhs)) > { > --- gcc/testsuite/g++.dg/cpp1y/constexpr-89074-2.C.jj 2022-01-06 20:51:52.327080068 +0100 > +++ gcc/testsuite/g++.dg/cpp1y/constexpr-89074-2.C 2022-01-06 20:51:18.338566365 +0100 > @@ -0,0 +1,19 @@ > +// PR c++/89074 > +// { dg-do compile { target c++14 } } > + > +constexpr bool > +foo () > +{ > + int a[] = { 1, 2 }; > + int b[] = { 3, 4 }; > + > + if (a + 0 == b + 0) > + return false; > + > + if (a + 1 == b + 0) > + return false; > + > + return true; > +} > + > +static_assert (foo (), ""); > --- gcc/testsuite/g++.dg/cpp1z/constexpr-89074-1.C.jj 2022-01-06 20:55:33.204919807 +0100 > +++ gcc/testsuite/g++.dg/cpp1z/constexpr-89074-1.C 2022-01-06 20:55:12.566215101 +0100 > @@ -0,0 +1,26 @@ > +// PR c++/89074 > +// { dg-do compile { target c++17 } } > + > +struct S { int s; }; > +struct T : public S { }; > +struct U : public T { }; > + > +constexpr bool > +foo () > +{ > + U a[] = { 1, 2, 3, 4 }; > + U b[] = { 5, 6, 7, 8 }; > + T *c = (T *) a + 1; > + S *d = (S *) c + 2; > + S *e = (S *) b + 1; > + > + if (a + 0 == b + 0) > + return false; > + > + if (d == e) > + return false; > + > + return true; > +} > + > +static_assert (foo (), ""); > > Jakub >