public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc(refs/vendors/ARM/heads/morello)] Store bound on COMPARE groups as non-capability
@ 2023-09-11 17:25 Matthew Malcomson
  0 siblings, 0 replies; only message in thread
From: Matthew Malcomson @ 2023-09-11 17:25 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:a1137fdd85d3b86c5034db0a6b8a73cd849d307a

commit a1137fdd85d3b86c5034db0a6b8a73cd849d307a
Author: Matthew Malcomson <matthew.malcomson@arm.com>
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 + <constant>)`.
    
    While this expression is valid, it can have a constant such that the
    sub-expression `&x + <constant>` 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) + <constant>`.
       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 + <constant>`
       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);

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2023-09-11 17:25 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-11 17:25 [gcc(refs/vendors/ARM/heads/morello)] Store bound on COMPARE groups as non-capability Matthew Malcomson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).