Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-August/578135.html
Are there any further comments on the patch?
Richard, you were kind enough to review the first two patches in this
series. Would you mind doing the same for this one? It continues in
the same vein.
Martin
On 8/25/21 3:26 PM, Martin Sebor wrote:
On 8/25/21 10:14 AM, Andrew MacLeod wrote:
On 8/25/21 11:20 AM, Martin Sebor wrote:
Ping: Andrew, did I answer your questions? Do you (or anyone else)
have any other comments on the latest patch below?
https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577865.html
I wasn't attempting to block it, its outside my review purview..
I was merely commenting that you should not need a pointer to a
range_query at all anymore
Are you planning to transition to using the get_range_query()
interface instead of keeping a range_query pointer in the
pointer_query class?
This pass and to a smaller extent the pointer_query class that's
used by it and the strlen pass are still a work in progress.
I also still need to convert the strlen pass to use Ranger and
I expect it will take some changes to pointer_query. So at that
point, if going through get_range_query (cfun) everywhere is what
you recommend, I'm happy to do it.
absolutely. you should not need to even know whether you have a ranger
instance running or not. get_range_query will give you whichever is
active, and there is ALWAYS something active.. defaulting to the
global version.
the code in get_range() seems to be the end of the call chain which
uses the pointer and should be consolidated to something much simpler
if (rvals && stmt)
{
if (!rvals->range_of_expr (vr, val, stmt))
return NULL_TREE;
<...>
// ?? This entire function should use get_range_query or
get_global_range_query (),
// instead of doing something different for RVALS and global
ranges.
if (!get_global_range_query ()->range_of_expr (vr, val) ||
vr.undefined_p ())
return NULL_TREE;
This entire section can boil down to something like
if (!get_range_query (cfun)->range_of_expr (vr, val, stmt))
return NULL;
So get_range_query (cfun) will never be null? And no special handling
of INTEGER_CST is needed, or checking for SSA_NAME. Nice. Attached
is another revision of the same patch with this function simplified.
(FYI: I expect the whole function to eventually go away, and to make
other similar simplifications, but I'm not there yet.)
PS There has been an effort to get rid of global variables from GCC,
or, as the first step, to avoid accessing them directly(*). If and
when that happens, it seems like each pass will have to store either
the ranger instance as a member (directly or indirectly, via a member
of a class that stores it) or the function passed to pass::execute()
if it wants to access either.
[*] https://gcc.gnu.org/pipermail/gcc-patches/2021-June/573975.html
The patch at the link above wasn't approved but IIUC removing globals
from GCC is still a goal.
I have no idea what direction that is going, but the net effect will
be the same in the end. You'll be calling get_range_query() with
either the function pointer, or the pass pointer, or something.. but
you should never need to pass around a pointer to either a ranger or
range-query. As I said earlier, this will likely be a pass property
and accessed thru the pass eventually... but until then, use what we
have.. cfun. It'll be trivial to transition down the road to whatever
the ultimate solution is.
Okay, I'll do that in my future changes.
Martin
