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).