[NewGVN] FIx phi-of-ops in the presence of memory read operations

The phi-of-ops functionality has a function OpIsSafeForPHIOfOps
to determine when it's safe to create the new phi.
But this function only checks for the obvious dominator conditions
and ignores memory.
This patch takes the conservative approach and disables phi-of-ops
whenever there's a load that doesn't dominate the phi, as its
value may be affected by a store inside the loop.

This can be improved later to check aliasing between the
load/stores.

Fixes https://llvm.org/PR53277

Reviewed By: asbirlea

Differential Revision: https://reviews.llvm.org/D117999
This commit is contained in:
Nuno Lopes 2022-01-26 10:10:22 +00:00
parent 66c602be25
commit 24a49e99f3
3 changed files with 86 additions and 28 deletions

View File

@ -2587,6 +2587,15 @@ bool NewGVN::OpIsSafeForPHIOfOpsHelper(
}
auto *OrigI = cast<Instruction>(V);
// When we hit an instruction that reads memory (load, call, etc), we must
// consider any store that may happen in the loop. For now, we assume the
// worst: there is a store in the loop that alias with this read.
// The case where the load is outside the loop is already covered by the
// dominator check above.
// TODO: relax this condition
if (OrigI->mayReadFromMemory())
return false;
for (auto *Op : OrigI->operand_values()) {
if (!isa<Instruction>(Op))
continue;

View File

@ -1,17 +1,55 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -passes=newgvn -enable-phi-of-ops=true -S -o - %s | FileCheck %s
define void @hoisted-load(i8* %p, i8* %q) {
; CHECK-LABEL: @hoisted-load(
; CHECK-NEXT: bb56:
; CHECK-NEXT: [[N60:%.*]] = load i8, i8* [[P:%.*]], align 1
; CHECK-NEXT: br label [[BB57:%.*]]
; CHECK: bb57:
; CHECK-NEXT: [[PHIOFOPS:%.*]] = phi i1 [ true, [[BB56:%.*]] ], [ [[N62:%.*]], [[BB229:%.*]] ]
; CHECK-NEXT: [[N59:%.*]] = phi i1 [ false, [[BB229]] ], [ true, [[BB56]] ]
; CHECK-NEXT: [[IDX:%.*]] = phi i8 [ 0, [[BB56]] ], [ [[INC:%.*]], [[BB229]] ]
; CHECK-NEXT: [[N62]] = icmp ne i8 [[N60]], 2
; CHECK-NEXT: br i1 [[PHIOFOPS]], label [[BB229]], label [[BB237:%.*]]
; CHECK: bb229:
; CHECK-NEXT: [[INC]] = add i8 [[IDX]], 1
; CHECK-NEXT: store i8 [[INC]], i8* [[Q:%.*]], align 1
; CHECK-NEXT: br label [[BB57]]
; CHECK: bb237:
; CHECK-NEXT: ret void
;
bb56:
%n60 = load i8, i8* %p
br label %bb57
bb57:
%n59 = phi i1 [ false, %bb229 ], [ true, %bb56 ]
%idx = phi i8 [0, %bb56], [%inc, %bb229]
%n62 = icmp ne i8 %n60, 2
%n63 = or i1 %n59, %n62
br i1 %n63, label %bb229, label %bb237
bb229:
%inc = add i8 %idx, 1
store i8 %inc, i8* %q
br label %bb57
bb237:
ret void
}
define void @store-in-loop(i8* %p, i8* %q) {
; CHECK-LABEL: @store-in-loop(
; CHECK-NEXT: bb56:
; CHECK-NEXT: br label [[BB57:%.*]]
; CHECK: bb57:
; CHECK-NEXT: [[PHIOFOPS:%.*]] = phi i1 [ true, [[BB56:%.*]] ], [ [[N62:%.*]], [[BB229:%.*]] ]
; CHECK-NEXT: [[N59:%.*]] = phi i1 [ false, [[BB229]] ], [ true, [[BB56]] ]
; CHECK-NEXT: [[N59:%.*]] = phi i1 [ false, [[BB229:%.*]] ], [ true, [[BB56:%.*]] ]
; CHECK-NEXT: [[IDX:%.*]] = phi i8 [ 0, [[BB56]] ], [ [[INC:%.*]], [[BB229]] ]
; CHECK-NEXT: [[N60:%.*]] = load i8, i8* [[P:%.*]], align 1
; CHECK-NEXT: [[N62]] = icmp ne i8 [[N60]], 2
; CHECK-NEXT: br i1 [[PHIOFOPS]], label [[BB229]], label [[BB237:%.*]]
; CHECK-NEXT: [[N62:%.*]] = icmp ne i8 [[N60]], 2
; CHECK-NEXT: [[N63:%.*]] = or i1 [[N59]], [[N62]]
; CHECK-NEXT: br i1 [[N63]], label [[BB229]], label [[BB237:%.*]]
; CHECK: bb229:
; CHECK-NEXT: [[INC]] = add i8 [[IDX]], 1
; CHECK-NEXT: store i8 [[INC]], i8* [[Q:%.*]], align 1
@ -39,17 +77,18 @@ bb237:
ret void
}
; TODO: we should support this case
define void @no-alias-store-in-loop(i8* noalias %p, i8* noalias %q) {
; CHECK-LABEL: @no-alias-store-in-loop(
; CHECK-NEXT: bb56:
; CHECK-NEXT: br label [[BB57:%.*]]
; CHECK: bb57:
; CHECK-NEXT: [[PHIOFOPS:%.*]] = phi i1 [ true, [[BB56:%.*]] ], [ [[N62:%.*]], [[BB229:%.*]] ]
; CHECK-NEXT: [[N59:%.*]] = phi i1 [ false, [[BB229]] ], [ true, [[BB56]] ]
; CHECK-NEXT: [[N59:%.*]] = phi i1 [ false, [[BB229:%.*]] ], [ true, [[BB56:%.*]] ]
; CHECK-NEXT: [[IDX:%.*]] = phi i8 [ 0, [[BB56]] ], [ [[INC:%.*]], [[BB229]] ]
; CHECK-NEXT: [[N60:%.*]] = load i8, i8* [[P:%.*]], align 1
; CHECK-NEXT: [[N62]] = icmp ne i8 [[N60]], 2
; CHECK-NEXT: br i1 [[PHIOFOPS]], label [[BB229]], label [[BB237:%.*]]
; CHECK-NEXT: [[N62:%.*]] = icmp ne i8 [[N60]], 2
; CHECK-NEXT: [[N63:%.*]] = or i1 [[N59]], [[N62]]
; CHECK-NEXT: br i1 [[N63]], label [[BB229]], label [[BB237:%.*]]
; CHECK: bb229:
; CHECK-NEXT: [[INC]] = add i8 [[IDX]], 1
; CHECK-NEXT: store i8 [[INC]], i8* [[Q:%.*]], align 1
@ -82,11 +121,11 @@ define void @function-in-loop(i8* %p) {
; CHECK-NEXT: bb56:
; CHECK-NEXT: br label [[BB57:%.*]]
; CHECK: bb57:
; CHECK-NEXT: [[PHIOFOPS:%.*]] = phi i1 [ true, [[BB56:%.*]] ], [ [[N62:%.*]], [[BB229:%.*]] ]
; CHECK-NEXT: [[N59:%.*]] = phi i1 [ false, [[BB229]] ], [ true, [[BB56]] ]
; CHECK-NEXT: [[N59:%.*]] = phi i1 [ false, [[BB229:%.*]] ], [ true, [[BB56:%.*]] ]
; CHECK-NEXT: [[N60:%.*]] = load i8, i8* [[P:%.*]], align 1
; CHECK-NEXT: [[N62]] = icmp ne i8 [[N60]], 2
; CHECK-NEXT: br i1 [[PHIOFOPS]], label [[BB229]], label [[BB237:%.*]]
; CHECK-NEXT: [[N62:%.*]] = icmp ne i8 [[N60]], 2
; CHECK-NEXT: [[N63:%.*]] = or i1 [[N59]], [[N62]]
; CHECK-NEXT: br i1 [[N63]], label [[BB229]], label [[BB237:%.*]]
; CHECK: bb229:
; CHECK-NEXT: call void @f()
; CHECK-NEXT: br label [[BB57]]
@ -111,16 +150,17 @@ bb237:
ret void
}
; TODO: we should support this case
define void @nowrite-function-in-loop(i8* %p) {
; CHECK-LABEL: @nowrite-function-in-loop(
; CHECK-NEXT: bb56:
; CHECK-NEXT: br label [[BB57:%.*]]
; CHECK: bb57:
; CHECK-NEXT: [[PHIOFOPS:%.*]] = phi i1 [ true, [[BB56:%.*]] ], [ [[N62:%.*]], [[BB229:%.*]] ]
; CHECK-NEXT: [[N59:%.*]] = phi i1 [ false, [[BB229]] ], [ true, [[BB56]] ]
; CHECK-NEXT: [[N59:%.*]] = phi i1 [ false, [[BB229:%.*]] ], [ true, [[BB56:%.*]] ]
; CHECK-NEXT: [[N60:%.*]] = load i8, i8* [[P:%.*]], align 1
; CHECK-NEXT: [[N62]] = icmp ne i8 [[N60]], 2
; CHECK-NEXT: br i1 [[PHIOFOPS]], label [[BB229]], label [[BB237:%.*]]
; CHECK-NEXT: [[N62:%.*]] = icmp ne i8 [[N60]], 2
; CHECK-NEXT: [[N63:%.*]] = or i1 [[N59]], [[N62]]
; CHECK-NEXT: br i1 [[N63]], label [[BB229]], label [[BB237:%.*]]
; CHECK: bb229:
; CHECK-NEXT: call void @f() #[[ATTR0:[0-9]+]]
; CHECK-NEXT: br label [[BB57]]

View File

@ -12,12 +12,16 @@ define i32 @foo(i32*, i32) {
; CHECK-NEXT: store i32 5, i32* [[TMP0:%.*]], align 4
; CHECK-NEXT: [[TMP3:%.*]] = icmp ne i32 [[TMP1:%.*]], 0
; CHECK-NEXT: br i1 [[TMP3]], label [[TMP4:%.*]], label [[TMP5:%.*]]
; CHECK: br label [[TMP5]]
; CHECK: [[PHIOFOPS:%.*]] = phi i32 [ 10, [[TMP2:%.*]] ], [ 15, [[TMP4]] ]
; CHECK-NEXT: [[DOT0:%.*]] = phi i32 [ 10, [[TMP4]] ], [ 5, [[TMP2]] ]
; CHECK-NEXT: br i1 [[TMP3]], label [[TMP6:%.*]], label [[TMP7:%.*]]
; CHECK: br label [[TMP7]]
; CHECK: [[DOT1:%.*]] = phi i32 [ [[PHIOFOPS]], [[TMP6]] ], [ [[DOT0]], [[TMP5]] ]
; CHECK: 4:
; CHECK-NEXT: br label [[TMP5]]
; CHECK: 5:
; CHECK-NEXT: [[DOT0:%.*]] = phi i32 [ 10, [[TMP4]] ], [ 5, [[TMP2:%.*]] ]
; CHECK-NEXT: br i1 [[TMP3]], label [[TMP6:%.*]], label [[TMP8:%.*]]
; CHECK: 6:
; CHECK-NEXT: [[TMP7:%.*]] = add nsw i32 [[DOT0]], 5
; CHECK-NEXT: br label [[TMP8]]
; CHECK: 8:
; CHECK-NEXT: [[DOT1:%.*]] = phi i32 [ [[TMP7]], [[TMP6]] ], [ [[DOT0]], [[TMP5]] ]
; CHECK-NEXT: ret i32 [[DOT1]]
;
store i32 5, i32* %0, align 4
@ -52,13 +56,18 @@ define i32 @foo2(i32*, i32) {
; CHECK-NEXT: store i32 5, i32* [[TMP0:%.*]], align 4
; CHECK-NEXT: [[TMP3:%.*]] = icmp ne i32 [[TMP1:%.*]], 0
; CHECK-NEXT: br i1 [[TMP3]], label [[TMP4:%.*]], label [[TMP5:%.*]]
; CHECK: br label [[TMP6:%.*]]
; CHECK: br label [[TMP6]]
; CHECK: [[PHIOFOPS:%.*]] = phi i32 [ 10, [[TMP5]] ], [ 15, [[TMP4]] ]
; CHECK: 4:
; CHECK-NEXT: br label [[TMP6:%.*]]
; CHECK: 5:
; CHECK-NEXT: br label [[TMP6]]
; CHECK: 6:
; CHECK-NEXT: [[DOT0:%.*]] = phi i32 [ 10, [[TMP4]] ], [ 5, [[TMP5]] ]
; CHECK-NEXT: br i1 [[TMP3]], label [[TMP7:%.*]], label [[TMP8:%.*]]
; CHECK: br label [[TMP8]]
; CHECK: [[DOT1:%.*]] = phi i32 [ [[PHIOFOPS]], [[TMP7]] ], [ [[DOT0]], [[TMP6]] ]
; CHECK-NEXT: br i1 [[TMP3]], label [[TMP7:%.*]], label [[TMP9:%.*]]
; CHECK: 7:
; CHECK-NEXT: [[TMP8:%.*]] = add nsw i32 [[DOT0]], 5
; CHECK-NEXT: br label [[TMP9]]
; CHECK: 9:
; CHECK-NEXT: [[DOT1:%.*]] = phi i32 [ [[TMP8]], [[TMP7]] ], [ [[DOT0]], [[TMP6]] ]
; CHECK-NEXT: ret i32 [[DOT1]]
;
store i32 5, i32* %0, align 4