diff --git a/llvm/lib/Transforms/Scalar/NewGVN.cpp b/llvm/lib/Transforms/Scalar/NewGVN.cpp index fc0b31c433962..e39bc23851bf7 100644 --- a/llvm/lib/Transforms/Scalar/NewGVN.cpp +++ b/llvm/lib/Transforms/Scalar/NewGVN.cpp @@ -2597,34 +2597,63 @@ bool NewGVN::OpIsSafeForPHIOfOps(Value *V, const BasicBlock *PHIBlock, Worklist.push_back(V); while (!Worklist.empty()) { auto *I = Worklist.pop_back_val(); - if (!isa(I)) + if (alwaysAvailable(I)) continue; + assert((isa(I) || isa(I)) && + "Should have been an Instruction or MemoryPhi"); auto OISIt = OpSafeForPHIOfOps.find({I, CacheIdx}); if (OISIt != OpSafeForPHIOfOps.end()) return OISIt->second; + auto *IBlock = getBlockForValue(I); // Keep walking until we either dominate the phi block, or hit a phi, or run // out of things to check. - if (DT->properlyDominates(getBlockForValue(I), PHIBlock)) { + if (DT->properlyDominates(IBlock, PHIBlock)) { OpSafeForPHIOfOps.insert({{I, CacheIdx}, true}); continue; } // PHI in the same block. - if (isa(I) && getBlockForValue(I) == PHIBlock) { + if ((isa(I) || isa(I)) && IBlock == PHIBlock) { OpSafeForPHIOfOps.insert({{I, CacheIdx}, false}); return false; } - auto *OrigI = cast(I); - // 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; + auto *OrigI = dyn_cast(I); + // When we encounter an instruction that reads memory (load, call, etc.) or + // a MemoryAccess, we must ensure that it does not depend on a memory Phi in + // PHIBlock. The base cases are already checked above; now we must check its + // operands. + MemoryAccess *MA = nullptr; + if (OrigI && OrigI->mayReadOrWriteMemory()) + MA = getMemoryAccess(OrigI); + else if (isa(I)) + MA = cast(I); + if (MA) { + for (auto *Op : MA->operand_values()) { + // Null => LiveOnEntryDef + if (!Op || MSSA->isLiveOnEntryDef(cast(Op))) + continue; + // Add the instruction that the memory access represents. + if (auto *MUD = dyn_cast(Op)) + Op = MUD->getMemoryInst(); + // Stop now if we find an unsafe operand. + auto OISIt = OpSafeForPHIOfOps.find({Op, CacheIdx}); + if (OISIt != OpSafeForPHIOfOps.end()) { + if (!OISIt->second) { + OpSafeForPHIOfOps.insert({{I, CacheIdx}, false}); + return false; + } + continue; + } + if (!Visited.insert(Op).second) + continue; + Worklist.push_back(Op); + } + } + // If it's a MemoryPhi there is nothing more to do. + if (isa(I)) + continue; // Check the operands of the current instruction. for (auto *Op : OrigI->operand_values()) { @@ -2641,7 +2670,7 @@ bool NewGVN::OpIsSafeForPHIOfOps(Value *V, const BasicBlock *PHIBlock, } if (!Visited.insert(Op).second) continue; - Worklist.push_back(cast(Op)); + Worklist.push_back(Op); } } OpSafeForPHIOfOps.insert({{V, CacheIdx}, true}); diff --git a/llvm/test/Transforms/NewGVN/phi-of-ops-loads.ll b/llvm/test/Transforms/NewGVN/phi-of-ops-loads.ll index 12c3389083cb2..2f1c2bb4efd51 100644 --- a/llvm/test/Transforms/NewGVN/phi-of-ops-loads.ll +++ b/llvm/test/Transforms/NewGVN/phi-of-ops-loads.ll @@ -77,18 +77,17 @@ bb237: ret void } -; TODO: we should support this case define void @no-alias-store-in-loop(ptr noalias %p, ptr noalias %q) { ; CHECK-LABEL: @no-alias-store-in-loop( ; CHECK-NEXT: bb56: ; CHECK-NEXT: br label [[BB57:%.*]] ; CHECK: bb57: -; CHECK-NEXT: [[N59:%.*]] = phi i1 [ false, [[BB229:%.*]] ], [ true, [[BB56:%.*]] ] +; 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: [[N60:%.*]] = load i8, ptr [[P:%.*]], align 1 -; 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-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]], ptr [[Q:%.*]], align 1 @@ -150,17 +149,16 @@ bb237: ret void } -; TODO: we should support this case define void @nowrite-function-in-loop(ptr %p) { ; CHECK-LABEL: @nowrite-function-in-loop( ; CHECK-NEXT: bb56: ; CHECK-NEXT: br label [[BB57:%.*]] ; CHECK: bb57: -; CHECK-NEXT: [[N59:%.*]] = phi i1 [ false, [[BB229:%.*]] ], [ true, [[BB56:%.*]] ] +; CHECK-NEXT: [[PHIOFOPS:%.*]] = phi i1 [ true, [[BB56:%.*]] ], [ [[N62:%.*]], [[BB229:%.*]] ] +; CHECK-NEXT: [[N59:%.*]] = phi i1 [ false, [[BB229]] ], [ true, [[BB56]] ] ; CHECK-NEXT: [[N60:%.*]] = load i8, ptr [[P:%.*]], align 1 -; 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-NEXT: [[N62]] = icmp ne i8 [[N60]], 2 +; CHECK-NEXT: br i1 [[PHIOFOPS]], label [[BB229]], label [[BB237:%.*]] ; CHECK: bb229: ; CHECK-NEXT: call void @f() #[[ATTR0:[0-9]+]] ; CHECK-NEXT: br label [[BB57]] @@ -199,9 +197,7 @@ define void @issfeoperand(ptr nocapture readonly %array, i1 %cond1, i1 %cond2, p ; CHECK-NEXT: [[PHI1:%.*]] = phi i8 [ [[LD1]], [[COND_TRUE]] ], [ 0, [[FOR_BODY:%.*]] ] ; CHECK-NEXT: [[ARRAYIDX42:%.*]] = getelementptr inbounds [3 x [2 x [1 x i8]]], ptr [[ARRAY]], i64 109, i64 0, i64 0, i64 undef ; CHECK-NEXT: [[LD2:%.*]] = load i8, ptr [[ARRAYIDX42]], align 1 -; CHECK-NEXT: [[CMP1:%.*]] = icmp ult i8 [[LD2]], [[PHI1]] -; CHECK-NEXT: [[ZEXT:%.*]] = zext i1 [[CMP1]] to i32 -; CHECK-NEXT: store i32 [[ZEXT]], ptr [[P2:%.*]], align 4 +; CHECK-NEXT: store i32 0, ptr [[P2:%.*]], align 4 ; CHECK-NEXT: br i1 [[COND2:%.*]], label [[COND_END:%.*]], label [[EXIT:%.*]] ; CHECK: cond.end: ; CHECK-NEXT: [[LD3:%.*]] = load i16, ptr [[P1:%.*]], align 2 diff --git a/llvm/test/Transforms/NewGVN/storeoverstore.ll b/llvm/test/Transforms/NewGVN/storeoverstore.ll index 0e31739b04e3e..2d74fe68a82d4 100644 --- a/llvm/test/Transforms/NewGVN/storeoverstore.ll +++ b/llvm/test/Transforms/NewGVN/storeoverstore.ll @@ -15,13 +15,13 @@ define i32 @foo(ptr, i32) { ; 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-NEXT: [[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: 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: br label [[TMP7]] +; CHECK: 7: +; CHECK-NEXT: [[DOT1:%.*]] = phi i32 [ [[PHIOFOPS]], [[TMP6]] ], [ [[DOT0]], [[TMP5]] ] ; CHECK-NEXT: ret i32 [[DOT1]] ; store i32 5, ptr %0, align 4