Re: [PATCH] middle-end, v4: IFN_ASSUME support [PR106654]

[Andrew MacLeod <amacleod@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4347 bytes --]


On 10/19/22 12:06, Jakub Jelinek wrote:
> Hi!
>
> On Tue, Oct 18, 2022 at 05:31:58PM -0400, Andrew MacLeod wrote:
>> Anyway, gives you something to experiement with.   If you would find a
>> different interface useful, let me know, or if there are limitations or
>> other expansions we might need.   This seems like something reasonable for
>> you to start working with?
> Thanks for working on this.
>
>> +  // Look for ASSUME calls, and call query_assume_call for each argument
>> +  // to determine if there is any inferred range to be had.
>> +  if (is_a<gcall *> (s) && gimple_call_internal_p (s)
>> +      && gimple_call_internal_fn (s) == IFN_ASSUME)
>> +    {
>> +      tree assume_id = gimple_call_arg (s, 0);
>> +      for (unsigned i = 1; i < gimple_call_num_args (s); i++)
>> +	{
>> +	  tree op = gimple_call_arg (s, i);
>> +	  tree type = TREE_TYPE (op);
>> +	  if (gimple_range_ssa_p (op) && Value_Range::supports_type_p (type))
>> +	    {
>> +	      Value_Range assume_range (type);
>> +	      if (query_assume_call (assume_range, assume_id, op))
>> +		{
>> +		  add_range (op, assume_range);
>> +		  if (dump_file)
>> +		    {
>> +		      print_generic_expr (dump_file, assume_id, TDF_SLIM);
>> +		      fprintf (dump_file, " assume inferred range of ");
>> +		      print_generic_expr (dump_file, op, TDF_SLIM);
>> +		      fprintf (dump_file, " to ");
>> +		      assume_range.dump (dump_file);
>> +		      fputc ('\n', dump_file);
>> +		    }
>> +		}
> Not sure I understand this part.  op is whatever we pass as the ith
> argument to IFN_ASSUME.  I'd expect that at this point one needs to
> remap that to the (i-1)th PARM_DECL of assume_id (so e.g. when you
> have the above loop you could as well start with DECL_ARGUMENTS and move
> that to DECL_CHAIN at the end of every iteration.  And then
> query ssa_default_def (DECL_STRUCT_FUNCTION (assume_id), parm)
> in each case and get global range of what that returns.

OK, this is the bit of code I dont know how to write :-)

yes, op is the name of the value within this current function, and yes, 
that needs to be mapped to the argument decl in the assume function.   
Then we need to query what range was given to that name during the 
assume pass.  when that is returned, the add_range (op, range) will 
inject it as a side effect.

Can you write that loop?

>
>> +  for (unsigned  x= 0; x < gimple_phi_num_args (phi); x++)
>    for (unsigned x = 0; ...
> ?
>
>> @@ -4345,6 +4345,30 @@ execute_ranger_vrp (struct function *fun, bool warn_array_bounds_p)
>>     scev_initialize ();
>>     calculate_dominance_info (CDI_DOMINATORS);
>>   
>> +  assume_query *r2 = new assume_query ();
>> +  for (unsigned i = 0; i < num_ssa_names; i++)
>> +    {
>> +      tree name = ssa_name (i);
>> +      if (!name || !gimple_range_ssa_p (name))
>> +	continue;
>> +      tree type = TREE_TYPE (name);
>> +      if (!Value_Range::supports_type_p (type))
>> +	continue;
>> +      Value_Range assume_range (type);
>> +      if (r2->assume_range_p (assume_range, name))
>> +	{
>> +	  if (dump_file)
>> +	    {
>> +	      fprintf (dump_file, "for an assume function, ");
>> +	      print_generic_expr (dump_file, name, TDF_SLIM);
>> +	      fprintf (dump_file, " would have a range of ");
>> +	      assume_range.dump (dump_file);
>> +	      fputc ('\n', dump_file);
>> +	    }
>> +	}
>> +    }
>> +  delete r2;
> I have expected (but tell me if that isn't possible) this could be something
> done in the new pass_assumptions::execute () rather than vrp and you'd
> create the assume_query there (i.e. just for assume_functions) and then
> query it solely for ssa_default_def of the parameters and save in
> SSA_NAME_RANGE_INFO.

I just discovered the assumption pass, and I have moved it to there.

I dont know much about managing the parameters, but presumably yes, we'd 
only query it for the parameters...........  I was showing the query for 
every name just to show what its producing.

As for storing it, SSA_NAME_RANGE_INFO is for the current function, that 
woud be easy.  if we store it there, how do we look up that range from 
another functi\on when we have the ASSUME_ID ? That was unclear to me.,


I've attached the replacement version of 0003* which uses the assume 
pass, and writes the global values out.  It still loops over all names 
at the moment

Andrew



>

[-- Attachment #2: 0003-Show-output-in-assume-pass.patch --]
[-- Type: text/x-patch, Size: 1439 bytes --]

From 655627144b8d8c02842d15ec83d720180231564a Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacleod@redhat.com>
Date: Mon, 17 Oct 2022 12:28:21 -0400
Subject: [PATCH 3/3] Show output in assume pass

---
 gcc/tree-vrp.cc | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/gcc/tree-vrp.cc b/gcc/tree-vrp.cc
index 1adb15c9934..3ec3d7daa74 100644
--- a/gcc/tree-vrp.cc
+++ b/gcc/tree-vrp.cc
@@ -4465,6 +4465,35 @@ public:
   bool gate (function *fun) final override { return fun->assume_function; }
   unsigned int execute (function *) final override
     {
+      assume_query query;
+      if (dump_file)
+	fprintf (dump_file, "Assumptions :\n--------------\n");
+      for (unsigned i = 0; i < num_ssa_names; i++)
+	{
+	  tree name = ssa_name (i);
+	  if (!name || !gimple_range_ssa_p (name))
+	    continue;
+	  tree type = TREE_TYPE (name);
+	  if (!Value_Range::supports_type_p (type))
+	    continue;
+	  Value_Range assume_range (type);
+	  if (query.assume_range_p (assume_range, name))
+	    {
+	      set_range_info (name, assume_range);
+	      if (dump_file)
+		{
+		  print_generic_expr (dump_file, name, TDF_SLIM);
+		  fprintf (dump_file, " -> ");
+		  assume_range.dump (dump_file);
+		  fputc ('\n', dump_file);
+		}
+	    }
+	}
+      if (dump_file)
+	{
+	  fputc ('\n', dump_file);
+	  gimple_dump_cfg (dump_file, dump_flags);
+	}
       return TODO_discard_function;
     }
 
-- 
2.37.3