[PATCH] D130482: [LAA] Avoid non-determinism due to blocks order. PR56672
Max Kazantsev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 25 22:51:20 PDT 2022
mkazantsev updated this revision to Diff 447571.
mkazantsev marked 5 inline comments as done.
mkazantsev added a comment.
Updated test as @fhahn has proposed and moved it to LAA tests. Floran, if you can take a look into it and confirm or disprove this is a correct fix, I'd appreciate that. I'm not familiar with LAA's algorithm, so I'm just seeing this as blackbox which behaves non-deterministically depending on blocks order which should not be the case. If there is a more targeted fix, that'd be nice. I'll hold this patch for a while.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130482/new/
https://reviews.llvm.org/D130482
Files:
llvm/lib/Analysis/LoopAccessAnalysis.cpp
llvm/test/Analysis/LoopAccessAnalysis/pr56672.ll
Index: llvm/test/Analysis/LoopAccessAnalysis/pr56672.ll
===================================================================
--- /dev/null
+++ llvm/test/Analysis/LoopAccessAnalysis/pr56672.ll
@@ -0,0 +1,39 @@
+; RUN: opt -passes='loop(loop-rotate),print-access-info' -S %s 2>&1 | FileCheck %s
+; RUN: opt -passes='loop(loop-rotate),invalidate<loops>,print-access-info' -S %s 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128-ni:1-p2:32:8:8:32-ni:2"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Make sure that vectorizer's behavior based on LAA remains the same regardless
+; of the order of blocks in loop.
+define void @test(i32* %p) {
+; CHECK-LABEL: test
+; CHECK: Report: unsafe dependent memory operations in loop.
+; CHECK-NOT: Memory dependences are safe
+bb:
+ br label %bb4
+
+bb1: ; preds = %bb4
+ br label %bb2
+
+bb2: ; preds = %bb1
+ store i32 1, i32* %tmp7, align 4
+ %tmp = add nuw i64 %tmp5, 1
+ %tmp3 = icmp ult i64 %tmp, 1000
+ br i1 %tmp3, label %bb4, label %bb11
+
+bb4: ; preds = %bb2, %bb
+ %tmp5 = phi i64 [ %tmp, %bb2 ], [ 16, %bb ]
+ %tmp6 = phi i64 [ %tmp5, %bb2 ], [ 15, %bb ]
+ %tmp7 = getelementptr inbounds i32, i32* %p, i64 %tmp5
+ %tmp8 = load i32, i32* %tmp7, align 4
+ %tmp9 = add i32 %tmp8, -5
+ store i32 %tmp9, i32* %tmp7, align 4
+ br i1 false, label %bb10, label %bb1
+
+bb10: ; preds = %bb4
+ unreachable
+
+bb11: ; preds = %bb2
+ ret void
+}
Index: llvm/lib/Analysis/LoopAccessAnalysis.cpp
===================================================================
--- llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -27,6 +27,7 @@
#include "llvm/Analysis/AliasSetTracker.h"
#include "llvm/Analysis/LoopAnalysisManager.h"
#include "llvm/Analysis/LoopInfo.h"
+#include "llvm/Analysis/LoopIterator.h"
#include "llvm/Analysis/MemoryLocation.h"
#include "llvm/Analysis/OptimizationRemarkEmitter.h"
#include "llvm/Analysis/ScalarEvolution.h"
@@ -2127,8 +2128,11 @@
EnableMemAccessVersioning &&
!TheLoop->getHeader()->getParent()->hasOptSize();
- // For each block.
- for (BasicBlock *BB : TheLoop->blocks()) {
+ // Traverse blocks in deterministic order, regardless of their storage in
+ // the loop info.
+ LoopBlocksRPO RPOT(TheLoop);
+ RPOT.perform(LI);
+ for (BasicBlock *BB : RPOT) {
// Scan the BB and collect legal loads and stores. Also detect any
// convergent instructions.
for (Instruction &I : *BB) {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D130482.447571.patch
Type: text/x-patch
Size: 2741 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220726/30326a83/attachment.bin>
More information about the llvm-commits
mailing list