Bug 97909 - expr_not_equal_to (mainly in match.pd) vs. ranger
Summary: expr_not_equal_to (mainly in match.pd) vs. ranger
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 11.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: internal-improvement
Depends on:
Blocks: VRP
  Show dependency treegraph
 
Reported: 2020-11-19 16:41 UTC by Jakub Jelinek
Modified: 2022-01-13 18:54 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelinek 2020-11-19 16:41:17 UTC
Something I've noticed today and filing this so that it isn't forgotten.
Various places in match.pd use expr_not_equal_to to decide based on value ranges whether to perform something or not.
This uses just SSA_NAME_RANGE_INFO under the hood right now.
Would be nice to use range for this; bet we'd need a way for match.pd to provide
extra information on where it is, so either a gimple *, or perhaps for match.pd it would be easier to provide a tree, which would be usually SSA_NAME of the lhs of the statement we want to query the range at, and then the function would check if the tree is an SSA_NAME and in that case talk to ranger and ask about range at the start of the SSA_NAME_DEF_STMT.
E.g. we have:
/* X % -Y is the same as X % Y.  */
(simplify
 (trunc_mod @0 (convert? (negate @1)))
 (if (INTEGRAL_TYPE_P (type)
      && !TYPE_UNSIGNED (type)
      && !TYPE_OVERFLOW_TRAPS (type)
      && tree_nop_conversion_p (type, TREE_TYPE (@1))
      /* Avoid this transformation if X might be INT_MIN or
         Y might be -1, because we would then change valid
         INT_MIN % -(-1) into invalid INT_MIN % -1.  */
      && (expr_not_equal_to (@0, wi::to_wide (TYPE_MIN_VALUE (type)))
          || expr_not_equal_to (@1, wi::minus_one (TYPE_PRECISION
                                                        (TREE_TYPE (@1))))))
  (trunc_mod @0 (convert @1))))
so if expr_not_equal_to has a defaulted = NULL_TREE extra argument, the second expr_not_equal_to could pass as the third argument @2 where it would be (negate@2 @1).  Unclear if we have something to pass for the trunc_mod's LHS or how to identify that statement.

Testcase for exactly this could be:
  int x = foo (), y;
  if (x != -1)
    y = z % -x;
  else
    y = -1; 
where we could optimize the y = z % -x; to y = z % x; but don't currently,
because we know that x is not -1 only in certain paths and SSA_NAME_RANGE_INFO can't reflect that.
Comment 1 Andrew Macleod 2020-11-19 18:07:03 UTC
Yeah, I'm planning as one of the next steps to replace the SSA_NAME_RANGE_INFO/get_range_info interface with the new range_query interface from value-query.h

Then we can wire that into the Ranger instance that a pass is using, if there is one.

Then any of these components, like this or the gimple-folder can make use of better range info if they know anything about the context in which they are working.

Once I finish going through these PRs, I plan to produce a work-list of all the mini-projects to complete each thing.  We can use this PR for the global range availability part.
Comment 2 Richard Biener 2020-11-20 07:09:17 UTC
Note that all folding may only ever access SSA use-def chains and cannot rely on immediate uses.  Also it may not use context (aka gimple_bb of def stmts) so I have a hard time seeing on-demand range compute to work from the folding context.
Comment 3 Andrew Macleod 2020-11-20 14:35:28 UTC
It should be able to access  the currently known global values that have been computed, or if there isn't one, it could still go and calculate a global range.   Which is what the default condition would be (ie, if you don't supply a context).

And we can explore under what conditions we can use more info... I suspect there are times it would be OK and we can supply a context only then.

Regardless, all the on-demand engine does is give you the same information you would have had if you calculated all of it before doing whatever you are about to do.   It just avoids the need to do so for anything that isn't relevant.

And if there are times that we can't calculate ranges, then we make sure we don't.
Comment 4 Andrew Macleod 2022-01-13 14:06:56 UTC
This functionality was added with fc4076752067fb400b43adbd629081df658da246

Commentary here https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583216.html

All one needs is an active ranger via the enable_ranger() API.
*All* queries made from within folding are READ_ONLY.. ie, no new information is ever created.  

So for accurate results, you need to query the ranges of any operands before invoking fold to make sure the caches and information are up to date.

Then ::fold_stmt needs to be invoked via ranger->fold_stmt() instead which provides a hook for the context required.

Any invocation of folding not done thru this interface will revert to whatever ranger knows about global ranges.

IF we wish to utilize this via different API points, we can add them.  

Ultimately, we can try to get tighter integration of context with folding.
Comment 5 GCC Commits 2022-01-13 18:52:22 UTC
The master branch has been updated by Andrew Macleod <amacleod@gcc.gnu.org>:

https://gcc.gnu.org/g:49d5fb4feee831868d80fff4d024c271911c92ca

commit r12-6559-g49d5fb4feee831868d80fff4d024c271911c92ca
Author: Andrew MacLeod <amacleod@redhat.com>
Date:   Wed Jan 12 13:31:08 2022 -0500

    Allow more precision when querying from fold_const.
    
    fold_const::expr_not_equal_to queries for a current range, but still uses
    the old value_range class.  This is causing it to miss opportunities when
    ranger can provide something better.
    
            PR tree-optimization/83072
            PR tree-optimization/83073
            PR tree-optimization/97909
            gcc/
            * fold-const.c (expr_not_equal_to): Use a multi-range class.
    
            gcc/testsuite/
            * gcc.dg/pr83072-2.c: New.
            * gcc.dg/pr83073.c: New.
Comment 6 Andrew Macleod 2022-01-13 18:54:58 UTC
This support is now available.