[PATCH] D33056: [NewGVN] When a store becomes the leader, update the memory leader for the class

[Davide Italiano via Phabricator via llvm-commits]

davide created this revision.
Herald added a subscriber: Prazek.

When we change the leader to a store:

  Changing leader of congruence class 6 from   %2 = load i32, i32* %d, align 4 to    store i32 %1, i32* %d, align 4 because store joined class

we don't update the memory leader so later, when trying to process a `load`

  Setting 2 = MemoryDef(6) equivalent to congruence class 6 with current MemoryAccess leader 7 = MemoryPhi({for.body,2},{if.then,3})
  Erasing expression 0x44014f0 from table
  Processing MemoryPhi 7 = MemoryPhi({for.body,2},{if.then,3})
  Memory Phi value numbered to 3 = MemoryDef(7)
  Setting 7 = MemoryPhi({for.body,2},{if.then,3}) equivalent to congruence class 8 with current MemoryAccess leader 3 = MemoryDef(7)
  Processing instruction   %2 = load i32, i32* %d, align 4

we crash while looking up the memory leader for the class

  opt: ../lib/Transforms/Scalar/NewGVN.cpp:1048: const llvm::MemoryAccess* {anonymous}::NewGVN::lookupMemoryLeader(const llvm::MemoryAccess*) const: Assertion `CC->getMemoryLeader() && "Every MemoryAccess should be mapped to a " "congruence class with a represenative memory " "access"' failed.


https://reviews.llvm.org/D33056

Files:
  lib/Transforms/Scalar/NewGVN.cpp
  test/Transforms/NewGVN/pr32934.ll


Index: test/Transforms/NewGVN/pr32934.ll
===================================================================
--- /dev/null
+++ test/Transforms/NewGVN/pr32934.ll
@@ -0,0 +1,68 @@
+; RUN: opt -S -newgvn %s | FileCheck %s
+
+; CHECK: define void @tinkywinky() {
+; CHECK-NEXT: entry:
+; CHECK-NEXT:   %d = alloca i32, align 4
+; CHECK-NEXT:   store i32 0, i32* null, align 4
+; CHECK-NEXT:   br label %for.cond
+; CHECK: for.cond:                                         ; preds = %if.end, %entry
+; CHECK-NEXT:   %0 = load i32, i32* null, align 4
+; CHECK-NEXT:   %cmp = icmp slt i32 %0, 1
+; CHECK-NEXT:   br i1 %cmp, label %for.body, label %while.cond
+; CHECK: for.body:                                         ; preds = %for.cond
+; CHECK-NEXT:   %1 = load i32, i32* @a, align 4
+; CHECK-NEXT:   store i32 %1, i32* %d, align 4
+; CHECK-NEXT:   br label %L
+; CHECK: L:                                                ; preds = %if.then, %for.body
+; CHECK-NEXT:   %tobool = icmp ne i32 %1, 0
+; CHECK-NEXT:   br i1 %tobool, label %if.then, label %if.end
+; CHECK: if.then:                                          ; preds = %L
+; CHECK-NEXT:   call void (i8*, ...) @printf(i8* getelementptr inbounds ([2 x i8], [2 x i8]* @patatino, i32 0, i32 0))
+; CHECK-NEXT:   br label %L
+; CHECK: if.end:                                           ; preds = %L
+; CHECK-NEXT:   br label %for.cond
+; CHECK: while.cond:                                       ; preds = %while.body, %for.cond
+; CHECK-NEXT:   br i1 undef, label %while.body, label %while.end
+; CHECK: while.body:                                       ; preds = %while.cond
+; CHECK-NEXT:   call void (i8*, ...) @printf(i8* getelementptr inbounds ([2 x i8], [2 x i8]* @patatino, i32 0, i32 0))
+; CHECK-NEXT:   br label %while.cond
+; CHECK: while.end:
+; CHECK-NEXT:   %2 = load i32, i32* @a, align 4
+; CHECK-NEXT:   store i32 %2, i32* undef, align 4
+; CHECK-NEXT:   ret void
+
+ at a = external global i32, align 4
+ at patatino = external unnamed_addr constant [2 x i8], align 1
+define void @tinkywinky() {
+entry:
+  %d = alloca i32, align 4
+  store i32 0, i32* null, align 4
+  br label %for.cond
+for.cond:
+  %0 = load i32, i32* null, align 4
+  %cmp = icmp slt i32 %0, 1
+  br i1 %cmp, label %for.body, label %while.cond
+for.body:
+  %1 = load i32, i32* @a, align 4
+  store i32 %1, i32* %d, align 4
+  br label %L
+L:
+  %2 = load i32, i32* %d, align 4
+  %tobool = icmp ne i32 %2, 0
+  br i1 %tobool, label %if.then, label %if.end
+if.then:
+  call void (i8*, ...) @printf(i8* getelementptr inbounds ([2 x i8], [2 x i8]* @patatino, i32 0, i32 0))
+  br label %L
+if.end:
+  br label %for.cond
+while.cond:
+  br i1 undef, label %while.body, label %while.end
+while.body:
+  call void (i8*, ...) @printf(i8* getelementptr inbounds ([2 x i8], [2 x i8]* @patatino, i32 0, i32 0))
+  br label %while.cond
+while.end:
+  %3 = load i32, i32* @a, align 4
+  store i32 %3, i32* undef, align 4
+  ret void
+}
+declare void @printf(i8*, ...) #1
Index: lib/Transforms/Scalar/NewGVN.cpp
===================================================================
--- lib/Transforms/Scalar/NewGVN.cpp
+++ lib/Transforms/Scalar/NewGVN.cpp
@@ -1991,8 +1991,10 @@
                      << NewClass->getID() << " from " << *NewClass->getLeader()
                      << " to  " << *SI << " because store joined class\n");
         // If we changed the leader, we have to mark it changed because we don't
-        // know what it will do to symbolic evlauation.
+        // know what it will do to symbolic evaluation.
         NewClass->setLeader(SI);
+        auto *StoreMA = dyn_cast_or_null<MemoryUse>(MSSA->getMemoryAccess(I));
+        NewClass->setMemoryLeader(StoreMA);
       }
       // We rely on the code below handling the MemoryAccess change.
     }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33056.98489.patch
Type: text/x-patch
Size: 3801 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170510/3f8a6194/attachment.bin>