This is the mail archive of the
mailing list for the GCC project.
Re: PR 28034: -fsection-anchors and coverage counters
- From: Richard Sandiford <richard at codesourcery dot com>
- To: Mark Mitchell <mark at codesourcery dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Wed, 21 Jun 2006 17:14:44 +0100
- Subject: Re: PR 28034: -fsection-anchors and coverage counters
- References: <email@example.com> <4498FEE1.firstname.lastname@example.org>
Mark Mitchell <email@example.com> writes:
> Richard Sandiford wrote:
>> /* Generate and save a copy of this so it can be shared. */
>> /* We don't know the size yet; make it big enough that nobody
>> will make any clever transformation on it. */
>> char buf;
>> tree gcov_type_node = get_gcov_type ();
>> tree domain_tree
>> = build_index_type (build_int_cst (NULL_TREE, 1000)); /* replaced later */
>> tree gcov_type_array_type
>> = build_array_type (gcov_type_node, domain_tree);
> I think this code should be considered wrong, in current GCC. Your
> concern about the optimizers getting confused if we access too many
> elements is reasonable. I don't think it's safe to rely on the type not
> being shared with other variables, either.
> I think it would be better to give the variable the type "T " (with no
> array bounds) at this point, and to leave DECL_SIZE set to NULL_TREE, as
> we would with "extern int i". Then, set the type, and DECL_SIZE, when
> the final size of the array is known.
> Do you think such a patch would be hard? If it is, then your patch
> certainly looks to be a reasonable fallback.
As discussed off-list, that was my first instinct too, but I was
reluctant to do it because
(a) It's a locally-defined object. I thought the optimisers might be
surprised that a locally-defined object had no known size.
(b) It almost seemed too obvious. I felt sure the original author
must have rejected it for some reason (perhaps (a)).
However, it bootstraps & regression tests on x86_64-linux-gnu without
problems, and it does fix the testcase. I also couldn't see a specific
reason for using a fixed size in the original patch submission:
So, OK to install?
Note: I used integer_type_node because the final index type is set
using build_int_cst (NULL, N). The final index type will therefore
be a variant of integer_type_node, so I thought the initial type should
be consistent with it.
* coverage.c (coverage_counter_alloc): Use an open index range for
the counter tables.
--- gcc/coverage.c (revision 114768)
+++ gcc/coverage.c (working copy)
@@ -388,13 +388,13 @@ coverage_counter_alloc (unsigned counter
- /* Generate and save a copy of this so it can be shared. */
- /* We don't know the size yet; make it big enough that nobody
- will make any clever transformation on it. */
+ /* Generate and save a copy of this so it can be shared. Leave
+ the array's upper bound unspecified for now; it will be set
+ after all functions have been compiled. */
tree gcov_type_node = get_gcov_type ();
- = build_index_type (build_int_cst (NULL_TREE, 1000)); /* replaced later */
+ = build_range_type (integer_type_node, integer_zero_node, NULL);
= build_array_type (gcov_type_node, domain_tree);