From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2049) id BCF36385841D; Mon, 11 Sep 2023 17:25:39 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BCF36385841D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1694453139; bh=ZHVd6lwPqnoVkbTzfBZ9+ZbRib1v/9L/dvBqCdnlHso=; h=From:To:Subject:Date:From; b=vulvJATD4KotfRsQ54dDTiGTfp547O6HkDjLrlaYOqtzQdGJiNR0cUBdzdtIiDWDx ZiX1M6NBpKqoEpTF803s6AGdtsfhL7PUTA5mIA8kldZxcAZqaZQiAHHdl2tFxN/+4t Lc9i7OQvF7Oo3thHDch5OGip7AGHXH+YM/n1czbo= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Matthew Malcomson To: gcc-cvs@gcc.gnu.org Subject: [gcc(refs/vendors/ARM/heads/morello)] Store bound on COMPARE groups as non-capability X-Act-Checkin: gcc X-Git-Author: Matthew Malcomson X-Git-Refname: refs/vendors/ARM/heads/morello X-Git-Oldrev: 7fc4b29813c02707bc8fef2bf22b6986847e0542 X-Git-Newrev: a1137fdd85d3b86c5034db0a6b8a73cd849d307a Message-Id: <20230911172539.BCF36385841D@sourceware.org> Date: Mon, 11 Sep 2023 17:25:39 +0000 (GMT) List-Id: https://gcc.gnu.org/g:a1137fdd85d3b86c5034db0a6b8a73cd849d307a commit a1137fdd85d3b86c5034db0a6b8a73cd849d307a Author: Matthew Malcomson Date: Mon Sep 11 16:18:58 2023 +0100 Store bound on COMPARE groups as non-capability `may_eliminate_iv` finds the bound of a comparison candidate in a loop. This is only ever used to replace the condition in a comparison. Since any condition will only ever be on the capability value rather than the full capability there is no particular need for this bound to be recorded as a capability typed expression. When this bound is used in a comparison expression, we then cast it to a capability value rather than a capability. This is done in `rewrite_use_compare`. This often results in a valid expression of the below form `(long unsigned int) (&x + )`. While this expression is valid, it can have a constant such that the sub-expression `&x + ` is an invalid capability. This becomes a problem if some later pass decides that this subexpression should be stored in the constant pool. The invalid capability being stored in the constant pool triggers a warning intended for user-specified invalid capability values getting written to memory. There are two main ways in which we could avoid this. 1) Rewrite the expression so that there is no invalid capability sub-expression in it. E.g. `((long unsigned int)&x) + `. Since there is no invalid capability sub-expression, this can not introduce any invalid capability constant in memory. 2) Avoid warnings when putting something into the constant pool on the assumption that this could be generated by the compiler rather than something that the user wrote. Cons of each choice: 1) Rewriting the expression seems to change the optimisation decisions of the compiler. Specifically for the main case we saw, rather than loading the address-value part of the expression `&x + ` from memory we load the address-value part of `&x` and then add a constant. This is to be expected -- the change in the sub-expressions was precisely in order to change this part of the codegen -- and the change may in some cases improve performance (if `&x` is already in the constant pool). However this seems to fall into the same category of deciding whether to map expressions with different offsets to the same base or not as discussed in the comment above `get_loop_invariant_expr`. That comment implies that this approach would be the less performant one. 2) Avoiding a warning on something in the constant pool could result in false negatives by avoiding warning on a constant that is present in the user code and represents a mistake said user might find useful to see. Honestly I don't see a huge difference in the two decisions. I'm taking approach (1) largely because I thought of it first, but the trade-off is between values which represent different preferences so would not be surprised by the disagreement. N.b. I have not actually looked into the feasibility of (2), expect one could simply add an argument to `assemble_capability` and pass that in the invocations used in `output_constant_pool_1` and below. After deciding to rewrite the expression, we had two obvious choices for where to make this adjustment. We could either make an adjustment in `rewrite_use_expression` by extracting the offset from the capability using `strip_offset` and replacing that capability base with the address value part of it, or we could make the adjustment earlier (when the "bound" of the loop is being recorded). It turned out easier to make the adjustment when the "bound" of the loop is being recorded since at that point it is still split up into an affine tree with the capability metadata by itself and separate offsets. Diff: --- gcc/tree-ssa-loop-ivopts.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c index a1017092f819..cbc1bd30bf47 100644 --- a/gcc/tree-ssa-loop-ivopts.c +++ b/gcc/tree-ssa-loop-ivopts.c @@ -5613,8 +5613,8 @@ may_eliminate_iv (struct ivopts_data *data, } cand_value_at (loop, cand, use->stmt, desc->niter, &bnd); - - *bound = fold_convert (TREE_TYPE (cand->iv->base), + aff_combination_drop_capability (&bnd); + *bound = fold_convert (noncapability_type (TREE_TYPE (cand->iv->base)), aff_combination_to_tree (&bnd)); *comp = iv_elimination_compare (data, use);