Bug 93917 - VRP forgets range of value read from memory
Summary: VRP forgets range of value read from memory
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 10.0
: P3 normal
Target Milestone: 13.0
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on:
Blocks: VRP
  Show dependency treegraph
 
Reported: 2020-02-24 23:31 UTC by Marc Glisse
Modified: 2023-09-19 15:18 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-02-25 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Glisse 2020-02-24 23:31:13 UTC
First a case that works:

void f(int n){
  if(n<0)__builtin_unreachable();
}

EVRP assigns a range to n, and VRP1 folds the comparison to false.

Let's add an indirection:

void f(int*n){
  if(*n<0)__builtin_unreachable();
}

EVRP still assigns a range to *n, but now VRP1 seems to forget that range, does not fold the comparison, and reassigns a range to *n. And again in VRP2. __builtin_unreachable survives all the way to FAB, which in my original code prevents any loop optimization. I know of plans to fold __builtin_unreachable earlier always, which would likely solve the issue, but independently of that, it seems that VRP is dropping some interesting information present in old ranges, possibly because a memory read is not considered an interesting statement.
Comment 1 Richard Biener 2020-02-25 09:40:57 UTC
VRP indeed doesn't start with SSA_NAME_RANGE_INFOs range but with UNDEF
and if not "interesting" it sets defs to varying which doesn't retain
the IL present range either.
Comment 2 Andrew Macleod 2020-11-19 23:20:25 UTC
This seems to be a good place to bring up a question I have.

What exactly do we want to with __builtin_unreachable across passes?

Ranger was initially starting with the previously set globally known value, and I had to disable it for pr61034.C

Basically, the routine was pre-optimized and inlined, and inlining sets the global range based on the output from the optimizations, and when ranger removed the code with the unreachable in it, the testcase failed.

so 
void f(int n){
  if(n<0)__builtin_unreachable();
}
n has a global range of [0, +INF]

Is that the basic point of this?  Can we set the global range to that and eliminate the if?

same thing for 
  if(*n<0)__builtin_unreachable();

I see:
    _1 = *n_3(D);
    if (_1 < 0)
      goto <bb 3>; [INV]
    else
      goto <bb 4>; [INV]

2->3  (T) _1 :  int [-INF, -1]
2->4  (F) _1 :  int [0, +INF]

=========== BB 3 ============
    <bb 3> :
    __builtin_unreachable ();

And we can know that _1 is [0, +INF] from then on. 

Can we kill all these unreachables() the first time we see them if we set the range appropriately?

If not, at what point do we want to kill them?
Comment 3 Andrew Macleod 2022-11-03 19:30:38 UTC
Fixed by
commit e7310e24b1c0ca67b1bb507c1330b2bf39e59e32
Author: Andrew MacLeod <amacleod@redhat.com>
Date:   Tue Oct 25 16:42:41 2022 -0400
    
    Make ranger vrp1 default.
Comment 4 Andrew Macleod 2023-09-19 15:11:15 UTC
Note a slight change in expectation as a result of commit r14-4141-gbf6b107e2a342319b3787ec960fc8014ef3aff91  for PR 110080

Due to a memory load in the second case, we do not remove the unreachable call now as there may be a commoning opportunity later (via inlining in this case) in which this unreachable call may provide new information.

The test case has been adjusted to current expectations where we leave this unreachable call and then remove it and set the global range in VRP2 instead.