Commit Graph

31 Commits

Author SHA1 Message Date
Taewook Oh 75acec8a14 Do not propagate DebugLoc across basic blocks
Summary:
DebugLoc shouldn't be propagated across basic blocks to prevent incorrect stepping and imprecise sample profile result. rL288903 addressed the wrong DebugLoc propagation issue by limiting the copy of DebugLoc when GVN removes a fully redundant load that is dominated by some other load. However, DebugLoc is still incorrectly propagated in the following example:


```
1:  extern int g;
2: 
3:  void foo(int x, int y, int z) {
4:    if (x)
5:      g = 0;
6:    else
7:      g = 1;
8:
9:    int i = 0;
10:   for ( ; i < y ; i++)
11:     if (i > z)
12:       g++;
13: }

```
Below is LLVM IR representation of the program before GVN:


```
@g = external local_unnamed_addr global i32, align 4

; Function Attrs: nounwind uwtable
define void @foo(i32 %x, i32 %y, i32 %z) local_unnamed_addr #0 !dbg !4 {
entry:
  %not.tobool = icmp eq i32 %x, 0, !dbg !8
  %.sink = zext i1 %not.tobool to i32, !dbg !8
  store i32 %.sink, i32* @g, align 4, !tbaa !9
  %cmp8 = icmp sgt i32 %y, 0, !dbg !13
  br i1 %cmp8, label %for.body.preheader, label %for.end, !dbg !17

for.body.preheader:                               ; preds = %entry
  br label %for.body, !dbg !19

for.body:                                         ; preds = %for.body.preheader, %for.inc
  %i.09 = phi i32 [ %inc4, %for.inc ], [ 0, %for.body.preheader ]
  %cmp1 = icmp sgt i32 %i.09, %z, !dbg !19
  br i1 %cmp1, label %if.then2, label %for.inc, !dbg !21

if.then2:                                         ; preds = %for.body
  %0 = load i32, i32* @g, align 4, !dbg !22, !tbaa !9
  %inc = add nsw i32 %0, 1, !dbg !22
  store i32 %inc, i32* @g, align 4, !dbg !22, !tbaa !9
  br label %for.inc, !dbg !23

for.inc:                                          ; preds = %for.body, %if.then2
  %inc4 = add nuw nsw i32 %i.09, 1, !dbg !24
  %exitcond = icmp ne i32 %inc4, %y, !dbg !13
  br i1 %exitcond, label %for.body, label %for.end.loopexit, !dbg !17

for.end.loopexit:                                 ; preds = %for.inc
  br label %for.end, !dbg !26

for.end:                                          ; preds = %for.end.loopexit, %entry
  ret void, !dbg !26
}

```
where 


```
!21 = !DILocation(line: 11, column: 9, scope: !15)
!22 = !DILocation(line: 12, column: 8, scope: !20)
!23 = !DILocation(line: 12, column: 7, scope: !20)
!24 = !DILocation(line: 10, column: 20, scope: !25)
```

And below is after GVN:


```
@g = external local_unnamed_addr global i32, align 4

define void @foo(i32 %x, i32 %y, i32 %z) local_unnamed_addr !dbg !4 {
entry:
  %not.tobool = icmp eq i32 %x, 0, !dbg !8
  %.sink = zext i1 %not.tobool to i32, !dbg !8
  store i32 %.sink, i32* @g, align 4, !tbaa !9
  %cmp8 = icmp sgt i32 %y, 0, !dbg !13
  br i1 %cmp8, label %for.body.preheader, label %for.end, !dbg !17

for.body.preheader:                               ; preds = %entry
  br label %for.body, !dbg !19

for.body:                                         ; preds = %for.inc, %for.body.preheader
  %0 = phi i32 [ %1, %for.inc ], [ %.sink, %for.body.preheader ], !dbg !21
  %i.09 = phi i32 [ %inc4, %for.inc ], [ 0, %for.body.preheader ]
  %cmp1 = icmp sgt i32 %i.09, %z, !dbg !19
  br i1 %cmp1, label %if.then2, label %for.inc, !dbg !22

if.then2:                                         ; preds = %for.body
  %inc = add nsw i32 %0, 1, !dbg !21
  store i32 %inc, i32* @g, align 4, !dbg !21, !tbaa !9
  br label %for.inc, !dbg !23

for.inc:                                          ; preds = %if.then2, %for.body
  %1 = phi i32 [ %inc, %if.then2 ], [ %0, %for.body ]
  %inc4 = add nuw nsw i32 %i.09, 1, !dbg !24
  %exitcond = icmp ne i32 %inc4, %y, !dbg !13
  br i1 %exitcond, label %for.body, label %for.end.loopexit, !dbg !17

for.end.loopexit:                                 ; preds = %for.inc
  br label %for.end, !dbg !26

for.end:                                          ; preds = %for.end.loopexit, %entry
  ret void, !dbg !26
}

```
As you see, GVN removes the load in if.then2 block and creates a phi instruction in for.body for it. The problem is that DebugLoc of remove load instruction is propagated to the newly created phi instruction, which is wrong. rL288903 cannot handle this case because ValuesPerBlock.size() is not 1 in this example when the load is removed.

Reviewers: aprantl, andreadb, wolfgangp

Reviewed By: andreadb

Subscribers: davide, llvm-commits

Differential Revision: https://reviews.llvm.org/D29254

llvm-svn: 293688
2017-01-31 20:57:13 +00:00
Daniel Berlin a53a72243a NewGVN: Instead of changeToUnreachable, insert an instruction SimplifyCFG will turn into unreachable when it runs
llvm-svn: 293515
2017-01-30 18:12:56 +00:00
Daniel Berlin e60a791748 Update pr31758.ll for unreachable revert
llvm-svn: 293502
2017-01-30 17:08:06 +00:00
Daniel Berlin e19f0e01a8 Revert "NewGVN: Make unreachable blocks be marked with unreachable"
This reverts commit r293196

Besides making things look nicer, ATM, we'd like to preserve analysis
more than we'd like to destroy the CFG.  We'll probably revisit in the future

llvm-svn: 293501
2017-01-30 17:06:55 +00:00
Daniel Berlin c479686af2 NewGVN: Add basic dead and redundant store elimination
Summary:
This adds basic dead and redundant store elimination to
NewGVN.  Unlike our current DSE, it will happily do cross-block DSE if
it meets our requirements.

We get a bunch of DSE's simple.ll cases, and some stuff it doesn't.
Unlike DSE, however, we only try to eliminate stores of the same value
to the same memory location, not just general stores to the same
memory location.

Reviewers: davide

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D29149

llvm-svn: 293258
2017-01-27 02:37:11 +00:00
Daniel Berlin 1ea5f324bd NewGVN: Fix bug exposed by PR31761
Summary:
This does not actually fix the testcase in PR31761 (discussion is
ongoing on the testcase), but does fix a bug it exposes, where stores
were not properly clobbering loads.

We accomplish this by unifying the memory equivalence infratructure
back into the normal congruence infrastructure, and then properly
destroying congruence classes when memory state leaders disappear.

Reviewers: davide

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D29195

llvm-svn: 293216
2017-01-26 22:21:48 +00:00
Daniel Berlin 66e3a3d0ac NewGVN: Fix output of pr31578 testcase now that we mark unreachable blocks as unreachable
llvm-svn: 293198
2017-01-26 18:49:03 +00:00
Daniel Berlin 2b83492eee NewGVN: Make unreachable blocks be marked with unreachable
llvm-svn: 293196
2017-01-26 18:30:29 +00:00
Davide Italiano ccbbc8313f [NewGVN] Skip uses in unreachable blocks.
Otherwise we ask for a domtree node that's not there, and we crash.

Differential Revision:  https://reviews.llvm.org/D29145

llvm-svn: 293122
2017-01-26 00:42:42 +00:00
Daniel Berlin 68af279533 NewGVN: Remove pr31686.ll, it is tested by pr31594.ll, which is much smaller and simpler
llvm-svn: 292649
2017-01-20 21:04:58 +00:00
Daniel Berlin 26addef1a0 NewGVN: Fix PR 31686 and PR 31698 by rewriting store leader handling.
Summary:

This rewrites store expression/leader handling.  We no longer use the
value operand as the leader, instead, we store it separately.  We also
now store the stored value as part of the expression, and compare it
when comparing stores for equality.  This enables us to get rid of a
bunch of our previous hacks and machinations, as the existing
machinery takes care of everything *except* updating the stored value
on classes.  The only time we have to update it is if the storecount
goes to 0, and when we do, we destroy it.

Since we no longer use the value operand as the leader, during elimination, we have to use the value operand.  Doing this also fixes a bunch of store forwarding cases we were missing.

Any value operand we use is guaranteed to either be updated by previous eliminations, or minimized by future ones.

(IE the fact that we don't use the most dominating value operand when it's not a constant does not affect anything).

Sadly, this change also exposes that we didn't pay attention to the
output of the pr31594.ll test, as it also very clearly exposes the
same store leader bug we are fixing here.

(I added pr31682.ll anyway, but maybe we think that's too large to be useful)

On the plus side, propagate-ir-flags.ll now passes due to the
corrected store forwarding.

This change was 3 stage'd on darwin and linux, with the full test-suite.

Reviewers:
davide
Subscribers:
llvm-commits

llvm-svn: 292648
2017-01-20 21:04:30 +00:00
Daniel Berlin 89fea6fd9d NewGVN: Fix PR 31682, an overactive assert.
Part of the assert has been left active for further debugging.
The other part has been turned into a stat for tracking for the
moment.

llvm-svn: 292583
2017-01-20 06:38:41 +00:00
Anna Thomas 698f0deea9 [AliasAnalysis] Fences do not modify constant memory location
Summary:
Fence instructions are currently marked as `ModRef` for all memory locations.

We can improve this for constant memory locations (such as constant globals),
since fence instructions cannot modify these locations.

This helps us to forward constant loads across fences (added test case in GVN).
There were no changes in behaviour for similar test cases in early-cse and licm.

Reviewers: dberlin, sanjoy, reames

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D28914

llvm-svn: 292546
2017-01-20 00:21:33 +00:00
Daniel Berlin 01831b7db2 NewGVN: Fix PR31613 test regex naming
llvm-svn: 291979
2017-01-13 23:54:10 +00:00
Daniel Berlin c0431fd02d NewGVN: Move leaders around properly to ensure we have a canonical dominating leader. Fixes PR 31613.
Summary:
This is a testcase where phi node cycling happens, and because we do
not order the leaders by domination or anything similar, the leader
keeps changing.

Using std::set for the members is too expensive, and we actually don't
need them sorted all the time, only at leader changes.

We could keep both a set and a vector, and keep them mostly sorted and
resort as necessary, or use a set and a fibheap, but all of this seems
premature.

After running some statistics, we are able to avoid the vast majority
of sorting by keeping a "next leader" field.  Most congruence classes only have
leader changes once or twice during GVN.

Reviewers: davide

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D28594

llvm-svn: 291968
2017-01-13 22:40:01 +00:00
Piotr Padlewski 9530883e8c [Devirtualization] MemDep returns non-local !invariant.group dependencies
Summary:
Memory Dependence Analysis was limited to return only local dependencies
for invariant.group handling. Now it returns NonLocal when it finds it
and then by asking getNonLocalPointerDependency we get found dep.

Thanks to this we are able to devirtualize loops!

    void indirect(A &a, int n) {
      for (int i = 0 ; i < n; i++)
        a.foo();

    }
    void test(int n) {
      A a;
      indirect(a);
    }

After inlining a.foo() will be changed to direct call, even if foo and A::A()
is external (but only if vtable definition is be available).

Reviewers: nlewycky, dberlin, chandlerc, rsmith

Subscribers: mehdi_amini, davide, llvm-commits

Differential Revision: https://reviews.llvm.org/D28137

llvm-svn: 291762
2017-01-12 11:33:58 +00:00
Daniel Berlin f6eba4be2c NewGVN: Fix PR31594, by tracking the store count of congruence
classes, and updating checking to allow for equivalence through
reachability.

(Sadly, the checking here is not perfect, and can't be made perfect,
so we'll have to disable it after we are satisfied with correctness.
Right now it is just "very unlikely" to happen.)

llvm-svn: 291698
2017-01-11 20:22:36 +00:00
Daniel Berlin b755aea8eb NewGVN: Fix PR 31573, a failure to verify memory congruency due to
not excluding ourselves when checking if any equivalent stores
exist.

llvm-svn: 291421
2017-01-09 05:34:29 +00:00
Piotr Padlewski 09ad678bc4 [MemDep] NFC walk invariant.group graph only down
Summary:
By using stripPointerCasts we can get to the root
value and then walk down the bitcast graph

Reviewers: reames

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D28181

llvm-svn: 291405
2017-01-08 22:26:06 +00:00
Daniel Berlin 32f8d560dd NewGVN: Make sure we properly lookup operand leaders while creating
congruence classes for stores, and then keep them up to date.  Add
testcases.

llvm-svn: 291351
2017-01-07 16:55:14 +00:00
Daniel Berlin d92e7f9f74 NewGVN: Fix PR 31501.
Summary: LLVM's non-standard notion of phi nodes means we can't both try to substitute for undef in phi nodes *and* use phi nodes as leaders all the time. This changes NewGVN to use the same semantics as SimplifyPHINode to decide which phi nodes are equivalent.

Reviewers: davide

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D28312

llvm-svn: 291308
2017-01-07 00:01:42 +00:00
Daniel Berlin aa0ec1e992 NewGVN: Add a test case for equivalent phis.
llvm-svn: 290830
2017-01-02 19:55:13 +00:00
Daniel Berlin 43a5f998df NewGVN: Add forgotten testcase for PR 31483
llvm-svn: 290829
2017-01-02 19:49:20 +00:00
Piotr Padlewski da36215017 [MemDep] Handle gep with zeros for invariant.group
Summary:
gep 0, 0 is equivalent to bitcast. LLVM canonicalizes it
to getelementptr because it make SROA can then handle it.

Simple case like

    void g(A &a) {
        z(a);
        if (glob)
            a.foo();
    }
    void testG() {
        A a;
        g(a);
    }

was not devirtualized with -fstrict-vtable-pointers because luck of
handling for gep 0 in Memory Dependence Analysis

Reviewers: dberlin, nlewycky, chandlerc

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D28126

llvm-svn: 290763
2016-12-30 18:45:07 +00:00
Daniel Berlin e0bd37e78f NewGVN: Fix PR 31491 by ensuring that we touch the right instructions. Change to one based numbering so we can assert we don't cause the same bug again.
llvm-svn: 290724
2016-12-29 22:15:12 +00:00
Daniel Berlin d59e8010c5 Don't use our own incorrect version of isTriviallyDeadInstruction in NewGVN. Fixes PR/31472
llvm-svn: 290549
2016-12-26 18:44:36 +00:00
Davide Italiano 8ea5e4fcae [NewGVN] Change test to reflect difference between GVN and NewGVN.
The current GVN algorithm folds unconditional branches to, it claims,
expose more PRE oportunities. The folding, if really needed,
(which is not sure, as it's not really proved it improves analysis)
can be done by an earlier cleanup pass instead of GVN itself.
Ack'ed/SGTM'd by Daniel Berlin.

Differential Revision:  https://reviews.llvm.org/D28117

llvm-svn: 290546
2016-12-26 18:10:09 +00:00
Daniel Berlin d7c12ee54c Value number stores and memory states so we can detect when memory states are equivalent (IE store of same value to memory).
Reviewers: davide

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D28084

llvm-svn: 290525
2016-12-25 22:23:49 +00:00
Chandler Carruth eb119ece4a Fix some DOS-style line endings that I suspect snuck in from one of the
frustrating Subversion clients that fails to do line ending translation
of text files.

llvm-svn: 290404
2016-12-23 02:02:26 +00:00
Davide Italiano e05e3306a3 [NewGVN] Add the pass to PassRegistry.def.
We need to hook up here to get it working with the new PM.
Add a test while here (and remove a typo).

llvm-svn: 290350
2016-12-22 16:35:02 +00:00
Davide Italiano 7e274e02ae [GVN] Initial check-in of a new global value numbering algorithm.
The code have been developed by Daniel Berlin over the years, and
the new implementation goal is that of addressing shortcomings of
the current GVN infrastructure, i.e. long compile time for large
testcases, lack of phi predication, no load/store value numbering
etc...

The current code just implements the "core" GVN algorithm, although
other pieces (load coercion, phi handling, predicate system) are
already implemented in a branch out of tree. Once the core is stable,
we'll start adding pieces on top of the base framework.
The test currently living in test/Transform/NewGVN are a copy
of the ones in GVN, with proper `XFAIL` (missing features in NewGVN).
A flag will be added in a future commit to enable NewGVN, so that
interested parties can exercise this code easily.

Differential Revision:  https://reviews.llvm.org/D26224

llvm-svn: 290346
2016-12-22 16:03:48 +00:00