From patchwork Wed Mar 29 15:22:48 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Bin.Cheng" X-Patchwork-Id: 744840 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 3vtWjr4KFQz9s0m for ; Thu, 30 Mar 2017 02:23:08 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="Znh//yWF"; 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 :mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; q=dns; s=default; b=ALKP4y0vx28p4L5 JEYcbnUCozThLVuh56hVDLSNnbz0OcFSmOwPr2SRb4AewIb+2ZXw27FiYn1vCEZh TevXwYlcuCXLrfENOVpjGQr8sEoIYviLVDK33d4ASdYMJU+Jd0g3J/0qRjkYSKxg IRKkLQjQ+WNsxfOnCX1zMxzTf8cE= 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 :mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; s=default; bh=nFsdBsvh0JW8wT84H6CS9 f7Jn7U=; b=Znh//yWFfEjf1zD/4qd0hgIXBKv4fLxG4OdbOicIBMk+cMVwstFkh qyDKJM1Tal8vFfiijWbzfIZzM18xH45zDjupH2+e5WcteloLyEJVALUQsi22aW/A lOlHtPmJ50va9gNyuZq5DhZezfAqJoNS41//bsNBvSv4C4cQWuZMik= Received: (qmail 49414 invoked by alias); 29 Mar 2017 15:22:53 -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 49401 invoked by uid 89); 29 Mar 2017 15:22:52 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.0 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-vk0-f46.google.com Received: from mail-vk0-f46.google.com (HELO mail-vk0-f46.google.com) (209.85.213.46) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 29 Mar 2017 15:22:49 +0000 Received: by mail-vk0-f46.google.com with SMTP id d188so21624579vka.0 for ; Wed, 29 Mar 2017 08:22:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=Rpcy6kplMZaML9eoji+n6rJiggGbWpQONfo87kzGWfA=; b=AdCFKWzYh6eS+CCWPJDGWO9Ln54Gbwrb1SKh3FXOdiBzLkeKNBy6fUu3Yq4YnJcKYo fz/hdZIn90i2j+IKRK4dQyArnzUCtMF9ztkYHE+t+ECd8gi7qpc1Yin/T1jv5jxD3ZM0 qjMz/7dnAzoP27RWOnoj7VMXh4leRs8joENFoFioaSRuYvIgRVvNhVH92C44ijCmZct+ cGAhAQqfX1AeeqkM++MEjfsUNmOmLTHeJj0vmwPxDX2Wr3aNgaTEPsSrdKI1lX8N+CfE 4MQecswdJpaRZ2fsQ4mT68fkSeMoEb8eNEk5B+U8pTJsgDPJI/9BoDYAzm5vZEYyoHHH Yv2A== X-Gm-Message-State: AFeK/H3bfVWTwwTHhcH3Whx9ohhQaOgtJiJuaIATxlgspzMuh7PlBSxjNmlSy4xSdvse0/Y2LLlgx5kyB+zxGg== X-Received: by 10.31.147.11 with SMTP id v11mr523787vkd.126.1490800969219; Wed, 29 Mar 2017 08:22:49 -0700 (PDT) MIME-Version: 1.0 Received: by 10.103.12.129 with HTTP; Wed, 29 Mar 2017 08:22:48 -0700 (PDT) In-Reply-To: References: From: "Bin.Cheng" Date: Wed, 29 Mar 2017 16:22:48 +0100 Message-ID: Subject: Re: [PATCH PR80153]Always generate folded type conversion in tree-affine To: Richard Biener Cc: Bin Cheng , "gcc-patches@gcc.gnu.org" , nd X-IsSubscribed: yes On Tue, Mar 28, 2017 at 1:34 PM, Richard Biener wrote: > On Tue, Mar 28, 2017 at 2:01 PM, Bin Cheng wrote: >> Hi, >> This patch is to fix PR80153. As analyzed in the PR, root cause is tree_affine lacks >> ability differentiating (unsigned)(ptr + offset) and (unsigned)ptr + (unsigned)offset, >> even worse, it always returns the former expression in aff_combination_tree, which >> is wrong if the original expression has the latter form. The patch resolves the issue >> by always returning the latter form expression, i.e, always trying to generate folded >> expression. Also as analyzed in comment, I think this change won't result in substantial >> code gen difference. >> I also need to adjust get_computation_aff for test case gcc.dg/tree-ssa/reassoc-19.c. >> Well, I think the changed behavior is correct, but for case the original pointer candidate >> is chosen, it should be unnecessary to compute in uutype. Also this adjustment only >> generates (unsigned)(pointer + offset) which is generated by tree-affine.c. >> Bootstrap and test on x86_64 and AArch64. Is it OK? > Thanks for reviewing. > Hmm. What is the desired goal? To have all elts added have > comb->type as type? Then > the type passed to add_elt_to_tree is redundant with comb->type. It > looks like it > is always passed comb->type now. Yes, except pointer type comb->type, elts are converted to comb->type with this patch. The redundant type is removed in updated patch. > > ISTR from past work in this area that it was important for pointer > combinations to allow > both pointer and sizetype elts at least. Yes, It's still important to allow different types for pointer and offset in pointer type comb. I missed a pointer type check condition in the patch, fixed in updated patch. > > Your change is incomplete I think, for the scale == -1 and POINTER_TYPE_P case > elt is sizetype now, not of pointer type. As said above, we are > trying to maintain > both pointer and sizetype elts with like: > > if (scale == 1) > { > if (!expr) > { > if (POINTER_TYPE_P (TREE_TYPE (elt))) > return elt; > else > return fold_convert (type1, elt); > } > > where your earilier fold to type would result in not all cases handled the same > (depending whether scale was -1 for example). IIUC, it doesn't matter. For comb->type being pointer type, the behavior remains the same. For comb->type being unsigned T, this elt is converted to ptr_offtype, rather than unsigned T, this doesn't matter because ptr_offtype and unsigned T are equal to each other, otherwise tree_to_aff_combination shouldn't distribute it as a single elt. Anyway, this is addressed in updated patch by checking pointer comb->type additionally. BTW, I think "scale==-1" case is a simple heuristic differentiating pointer_base and offset. > > Thus - shouldn't we simply drop the type argument (or rather the comb one? > that wide_int_ext_for_comb looks weird given we get a widest_int as input > and all the other wide_int_ext_for_comb calls around). > > And unconditionally convert to type, simplifying the rest of the code? As said, for pointer type comb, we need to keep current behavior; for other cases, unconditionally convert to comb->type is the goal. Bootstrap and test on x86_64 and AArch64. Is this version OK? Thanks, bin 2017-03-28 Bin Cheng PR tree-optimization/80153 * tree-affine.c (add_elt_to_tree): Remove parameter TYPE. Use type of parameter COMB. Convert elt to type of COMB it COMB is not of pointer type. (aff_combination_to_tree): Update calls to add_elt_to_tree. * tree-ssa-loop-ivopts.c (alloc_iv): Pass in consistent types. (get_computation_aff): Use utype directly for original candidate. gcc/testsuite/ChangeLog 2017-03-28 Bin Cheng PR tree-optimization/80153 * gcc.c-torture/execute/pr80153.c: New. diff --git a/gcc/testsuite/gcc.c-torture/execute/pr80153.c b/gcc/testsuite/gcc.c-torture/execute/pr80153.c new file mode 100644 index 0000000..3eed578 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr80153.c @@ -0,0 +1,48 @@ +/* PR tree-optimization/80153 */ + +void check (int, int, int) __attribute__((noinline)); +void check (int c, int c2, int val) +{ + if (!val) { + __builtin_abort(); + } +} + +static const char *buf; +static int l, i; + +void _fputs(const char *str) __attribute__((noinline)); +void _fputs(const char *str) +{ + buf = str; + i = 0; + l = __builtin_strlen(buf); +} + +char _fgetc() __attribute__((noinline)); +char _fgetc() +{ + char val = buf[i]; + i++; + if (i > l) + return -1; + else + return val; +} + +static const char *string = "oops!\n"; + +int main(void) +{ + int i; + int c; + + _fputs(string); + + for (i = 0; i < __builtin_strlen(string); i++) { + c = _fgetc(); + check(c, string[i], c == string[i]); + } + + return 0; +} diff --git a/gcc/tree-affine.c b/gcc/tree-affine.c index e620eea..a7f0b6a 100644 --- a/gcc/tree-affine.c +++ b/gcc/tree-affine.c @@ -370,27 +370,28 @@ tree_to_aff_combination (tree expr, tree type, aff_tree *comb) aff_combination_elt (comb, type, expr); } -/* Creates EXPR + ELT * SCALE in TYPE. EXPR is taken from affine +/* Creates EXPR + ELT * SCALE in COMB's type. EXPR is taken from affine combination COMB. */ static tree -add_elt_to_tree (tree expr, tree type, tree elt, const widest_int &scale_in, - aff_tree *comb ATTRIBUTE_UNUSED) +add_elt_to_tree (tree expr, tree elt, const widest_int &scale_in, + aff_tree *comb) { enum tree_code code; - tree type1 = type; - if (POINTER_TYPE_P (type)) - type1 = sizetype; + tree type = POINTER_TYPE_P (comb->type) ? sizetype : comb->type; widest_int scale = wide_int_ext_for_comb (scale_in, comb); if (scale == -1 + && POINTER_TYPE_P (comb->type) && POINTER_TYPE_P (TREE_TYPE (elt))) { elt = convert_to_ptrofftype (elt); elt = fold_build1 (NEGATE_EXPR, TREE_TYPE (elt), elt); scale = 1; } + else if (!POINTER_TYPE_P (comb->type)) + elt = fold_convert (comb->type, elt); if (scale == 1) { @@ -399,22 +400,20 @@ add_elt_to_tree (tree expr, tree type, tree elt, const widest_int &scale_in, if (POINTER_TYPE_P (TREE_TYPE (elt))) return elt; else - return fold_convert (type1, elt); + return fold_convert (type, elt); } if (POINTER_TYPE_P (TREE_TYPE (expr))) return fold_build_pointer_plus (expr, elt); if (POINTER_TYPE_P (TREE_TYPE (elt))) return fold_build_pointer_plus (elt, expr); - return fold_build2 (PLUS_EXPR, type1, - expr, fold_convert (type1, elt)); + return fold_build2 (PLUS_EXPR, type, expr, fold_convert (type, elt)); } if (scale == -1) { if (!expr) - return fold_build1 (NEGATE_EXPR, type1, - fold_convert (type1, elt)); + return fold_build1 (NEGATE_EXPR, type, fold_convert (type, elt)); if (POINTER_TYPE_P (TREE_TYPE (expr))) { @@ -422,14 +421,12 @@ add_elt_to_tree (tree expr, tree type, tree elt, const widest_int &scale_in, elt = fold_build1 (NEGATE_EXPR, TREE_TYPE (elt), elt); return fold_build_pointer_plus (expr, elt); } - return fold_build2 (MINUS_EXPR, type1, - expr, fold_convert (type1, elt)); + return fold_build2 (MINUS_EXPR, type, expr, fold_convert (type, elt)); } - elt = fold_convert (type1, elt); + elt = fold_convert (type, elt); if (!expr) - return fold_build2 (MULT_EXPR, type1, elt, - wide_int_to_tree (type1, scale)); + return fold_build2 (MULT_EXPR, type, elt, wide_int_to_tree (type, scale)); if (wi::neg_p (scale)) { @@ -439,15 +436,14 @@ add_elt_to_tree (tree expr, tree type, tree elt, const widest_int &scale_in, else code = PLUS_EXPR; - elt = fold_build2 (MULT_EXPR, type1, elt, - wide_int_to_tree (type1, scale)); + elt = fold_build2 (MULT_EXPR, type, elt, wide_int_to_tree (type, scale)); if (POINTER_TYPE_P (TREE_TYPE (expr))) { if (code == MINUS_EXPR) - elt = fold_build1 (NEGATE_EXPR, type1, elt); + elt = fold_build1 (NEGATE_EXPR, type, elt); return fold_build_pointer_plus (expr, elt); } - return fold_build2 (code, type1, expr, elt); + return fold_build2 (code, type, expr, elt); } /* Makes tree from the affine combination COMB. */ @@ -455,22 +451,18 @@ add_elt_to_tree (tree expr, tree type, tree elt, const widest_int &scale_in, tree aff_combination_to_tree (aff_tree *comb) { - tree type = comb->type; tree expr = NULL_TREE; unsigned i; widest_int off, sgn; - tree type1 = type; - if (POINTER_TYPE_P (type)) - type1 = sizetype; + tree type = POINTER_TYPE_P (comb->type) ? sizetype : comb->type; gcc_assert (comb->n == MAX_AFF_ELTS || comb->rest == NULL_TREE); for (i = 0; i < comb->n; i++) - expr = add_elt_to_tree (expr, type, comb->elts[i].val, comb->elts[i].coef, - comb); + expr = add_elt_to_tree (expr, comb->elts[i].val, comb->elts[i].coef, comb); if (comb->rest) - expr = add_elt_to_tree (expr, type, comb->rest, 1, comb); + expr = add_elt_to_tree (expr, comb->rest, 1, comb); /* Ensure that we get x - 1, not x + (-1) or x + 0xff..f if x is unsigned. */ @@ -484,8 +476,7 @@ aff_combination_to_tree (aff_tree *comb) off = comb->offset; sgn = 1; } - return add_elt_to_tree (expr, type, wide_int_to_tree (type1, off), sgn, - comb); + return add_elt_to_tree (expr, wide_int_to_tree (type, off), sgn, comb); } /* Copies the tree elements of COMB to ensure that they are not shared. */ diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c index 8dc65881..fa993ab 100644 --- a/gcc/tree-ssa-loop-ivopts.c +++ b/gcc/tree-ssa-loop-ivopts.c @@ -1171,7 +1171,7 @@ alloc_iv (struct ivopts_data *data, tree base, tree step, || contain_complex_addr_expr (expr)) { aff_tree comb; - tree_to_aff_combination (expr, TREE_TYPE (base), &comb); + tree_to_aff_combination (expr, TREE_TYPE (expr), &comb); base = fold_convert (TREE_TYPE (base), aff_combination_to_tree (&comb)); } @@ -3787,6 +3787,12 @@ get_computation_aff (struct loop *loop, overflows, as all the arithmetics will in the end be performed in UUTYPE anyway. */ common_type = determine_common_wider_type (&ubase, &cbase); + /* We don't need to compute in UUTYPE if this is the original candidate, + and candidate/use have the same (pointer) type. */ + if (ctype == utype && common_type == utype + && POINTER_TYPE_P (utype) && TYPE_UNSIGNED (utype) + && cand->pos == IP_ORIGINAL && cand->incremented_at == use->stmt) + uutype = utype; /* use = ubase - ratio * cbase + ratio * var. */ tree_to_aff_combination (ubase, common_type, aff);