On 5/25/21 3:04 AM, Richard Biener wrote:
On Tue, May 25, 2021 at 2:53 AM Martin Sebor via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
On 5/24/21 5:08 PM, David Malcolm wrote:
On Mon, 2021-05-24 at 16:02 -0600, Martin Sebor wrote:
The rare expressions that have no location
continue to have just one bit[1].
Where does this get stored? I see the final patch in the kit removes
TREE_NO_WARNING, but I don't quite follow the logic for where the bit
would then be stored for an expr with UNKNOWN_LOCATION.
The patch just removes the TREE_NO_WARNING macro (along with
the gimple_no_warning_p/gimple_set_no_warning) functions but not
the no-warning bit itself. It removes them to avoid accidentally
modifying the bit alone without going through the new API and
updating the location -> warning group mapping. The bit is still
needed for expression/statements with no location.
I wonder if we could clone UNKNOWN_LOCATION, thus when
we set_no_warning on UNKNOWN_LOCATION create a new location
with the source location being still UNKNOWN but with the appropriate
ad-hoc data to disable the warning? That of course requires the
API to be
location_t set_no_warning (...)
and users would need to update the container with the new location
(or we'd need to use a reference we can update in set_no_warning).
This could be done even in the new set_no_warning(tree, ...), without
changing the callers. But I think the right place and time to set
the location is in the code that creates the expression. Doing it
at the time the no-warning bit is being set, either in the new API
or in the caller, seems like papering over the underlying problem.
That said - do you have any stats on how many UNKNOWN_LOCATION
locations we run into with boostrap / the testsuite?
During stage1, roughly 2.7% of all calls to set_no_warning() are
with arguments with no location. Based on the code I've seen
some are to be expected (e.g. eh_filter_expr, try_catch_expr,
and var_decl for artificial temporaries). The rest are:
compound_expr, cond_expr, eh_filter_expr, imagpart_expr,
indirect_ref, mem_ref, minus_expr, modify_expr, mult_expr,
ne_expr, plus_expr, realpart_expr, try_catch_expr, and var_decl.
The vast majority (90%) are plus_expr. At least some of them come
from ASSERT_EXPRs. The location isn't available at the point VRP
calls set_no_warning() but it is available earlier when the ASSERTs
are being created from the COND_EXPR statements. I can look into
this when I'm done with this.
Martin
