From 50afd4a305970ba7a599c0906809ecdd659b68e9ffa3b358483d7dbb9de3e944 Mon Sep 17 00:00:00 2001 From: Aaron Puchert Date: Sat, 3 Sep 2022 16:28:17 +0000 Subject: [PATCH 1/2] - Make sure we keep -DNDEBUG. At some point %{optflags} must have lost it, perhaps because CMake usually adds it on top. So when overriding CMAKE_{C,CXX}_FLAGS_RELWITHDEBINFO, we make sure to take over the other flags. We drop LLVM_ENABLE_ASSERTIONS=OFF, because that's the default anyway and hasn't helped here. - Add llvm-scev-fix-isImpliedViaMerge.patch: fixes a miscompilation caused by mixing up values of the current and previous iteration. (See gh#llvm/llvm-project#56242.) OBS-URL: https://build.opensuse.org/package/show/devel:tools:compiler/llvm14?expand=0&rev=38 --- llvm-scev-fix-isImpliedViaMerge.patch | 155 ++++++++++++++++++++++++++ llvm14.changes | 12 ++ llvm14.spec | 9 +- 3 files changed, 172 insertions(+), 4 deletions(-) create mode 100644 llvm-scev-fix-isImpliedViaMerge.patch diff --git a/llvm-scev-fix-isImpliedViaMerge.patch b/llvm-scev-fix-isImpliedViaMerge.patch new file mode 100644 index 0000000..7bad43b --- /dev/null +++ b/llvm-scev-fix-isImpliedViaMerge.patch @@ -0,0 +1,155 @@ +From cfb4c1a735e9648ead5bf60a1fab4f09b5ee6453 Mon Sep 17 00:00:00 2001 +From: Nikita Popov +Date: Mon, 27 Jun 2022 14:54:03 +0200 +Subject: [PATCH] [IndVars] Add test for PR56242 (NFC) + +--- + .../test/Transforms/IndVarSimplify/pr56242.ll | 49 +++++++++++++++++++ + 1 file changed, 49 insertions(+) + create mode 100644 llvm/test/Transforms/IndVarSimplify/pr56242.ll + +diff --git a/llvm/test/Transforms/IndVarSimplify/pr56242.ll b/llvm/test/Transforms/IndVarSimplify/pr56242.ll +new file mode 100644 +index 0000000000000..82e1d2252e760 +--- /dev/null ++++ b/llvm/test/Transforms/IndVarSimplify/pr56242.ll +@@ -0,0 +1,49 @@ ++; NOTE: Assertions have been autogenerated by utils/update_test_checks.py ++; RUN: opt -S -indvars < %s | FileCheck %s ++ ++declare void @use(i1) ++ ++define void @test(ptr %arr) { ++; CHECK-LABEL: @test( ++; CHECK-NEXT: entry: ++; CHECK-NEXT: br label [[LOOP_HEADER:%.*]] ++; CHECK: loop.header: ++; CHECK-NEXT: [[IV:%.*]] = phi i64 [ [[IV_INC:%.*]], [[LOOP_LATCH:%.*]] ], [ 0, [[ENTRY:%.*]] ] ++; CHECK-NEXT: [[PTR:%.*]] = getelementptr inbounds i32, ptr [[ARR:%.*]], i64 [[IV]] ++; CHECK-NEXT: [[V:%.*]] = load i32, ptr [[PTR]], align 4 ++; CHECK-NEXT: [[CMP1:%.*]] = icmp sgt i32 [[V]], 0 ++; CHECK-NEXT: br i1 [[CMP1]], label [[IF:%.*]], label [[LOOP_LATCH]] ++; CHECK: if: ++; CHECK-NEXT: call void @use(i1 false) ++; CHECK-NEXT: br label [[LOOP_LATCH]] ++; CHECK: loop.latch: ++; CHECK-NEXT: [[IV_INC]] = add nuw nsw i64 [[IV]], 1 ++; CHECK-NEXT: [[CMP:%.*]] = icmp ult i64 [[IV_INC]], 16 ++; CHECK-NEXT: br i1 [[CMP]], label [[LOOP_HEADER]], label [[EXIT:%.*]] ++; CHECK: exit: ++; CHECK-NEXT: ret void ++; ++entry: ++ br label %loop.header ++ ++loop.header: ++ %iv = phi i64 [ %iv.inc, %loop.latch ], [ 0, %entry ] ++ %prev = phi i32 [ %v, %loop.latch ], [ 0, %entry ] ++ %ptr = getelementptr inbounds i32, ptr %arr, i64 %iv ++ %v = load i32, ptr %ptr ++ %cmp1 = icmp sgt i32 %v, 0 ++ br i1 %cmp1, label %if, label %loop.latch ++ ++if: ++ %cmp2 = icmp slt i32 %prev, 0 ++ call void @use(i1 %cmp2) ++ br label %loop.latch ++ ++loop.latch: ++ %iv.inc = add nuw nsw i64 %iv, 1 ++ %cmp = icmp ult i64 %iv.inc, 16 ++ br i1 %cmp, label %loop.header, label %exit ++ ++exit: ++ ret void ++} + +From e4d1d0cc2c9ca38f98bc9b70c3e3db3a18f1e06e Mon Sep 17 00:00:00 2001 +From: Nikita Popov +Date: Mon, 27 Jun 2022 15:09:24 +0200 +Subject: [PATCH] [SCEV] Fix isImpliedViaMerge() with values from previous + iteration (PR56242) + +When trying to prove an implied condition on a phi by proving it +for all incoming values, we need to be careful about values coming +from a backedge, as these may refer to a previous loop iteration. +A variant of this issue was fixed in D101829, but the dominance +condition used there isn't quite right: It checks that the value +dominates the incoming block, which doesn't exclude backedges +(values defined in a loop will usually dominate the loop latch, +which is the incoming block of the backedge). + +Instead, we should be checking for domination of the phi block. +Any values defined inside the loop will not dominate the loop +header phi. + +Fixes https://github.com/llvm/llvm-project/issues/56242. + +Differential Revision: https://reviews.llvm.org/D128640 +--- + llvm/lib/Analysis/ScalarEvolution.cpp | 2 +- + llvm/test/Transforms/IRCE/decrementing-loop.ll | 11 +++++------ + llvm/test/Transforms/IndVarSimplify/pr56242.ll | 6 ++++-- + 3 files changed, 10 insertions(+), 9 deletions(-) + +diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp +index aa349f9435ce9..968665c4d3876 100644 +--- a/llvm/lib/Analysis/ScalarEvolution.cpp ++++ b/llvm/lib/Analysis/ScalarEvolution.cpp +@@ -11812,7 +11812,7 @@ bool ScalarEvolution::isImpliedViaMerge(ICmpInst::Predicate Pred, + const SCEV *L = getSCEV(LPhi->getIncomingValueForBlock(IncBB)); + // Make sure L does not refer to a value from a potentially previous + // iteration of a loop. +- if (!properlyDominates(L, IncBB)) ++ if (!properlyDominates(L, LBB)) + return false; + if (!ProvedEasily(L, RHS)) + return false; +diff --git a/llvm/test/Transforms/IRCE/decrementing-loop.ll b/llvm/test/Transforms/IRCE/decrementing-loop.ll +index 5ae4412b34bd7..462607162bcbe 100644 +--- a/llvm/test/Transforms/IRCE/decrementing-loop.ll ++++ b/llvm/test/Transforms/IRCE/decrementing-loop.ll +@@ -210,17 +210,16 @@ exit: + ret void + } + +-; TODO: we need to be more careful when trying to look through phi nodes in +-; cycles, because the condition to prove may reference the previous value of +-; the phi. So we currently fail to optimize this case. + ; Check that we can figure out that IV is non-negative via implication through + ; two Phi nodes, one being AddRec. + define void @test_05(i32* %a, i32* %a_len_ptr, i1 %cond) { + + ; CHECK-LABEL: test_05 +-; CHECK: entry: +-; CHECK: br label %merge +-; CHECK-NOT: mainloop ++; CHECK: mainloop: ++; CHECK-NEXT: br label %loop ++; CHECK: loop: ++; CHECK: br i1 true, label %in.bounds, label %out.of.bounds ++; CHECK: loop.preloop: + + entry: + %len.a = load i32, i32* %a_len_ptr, !range !0 +diff --git a/llvm/test/Transforms/IndVarSimplify/pr56242.ll b/llvm/test/Transforms/IndVarSimplify/pr56242.ll +index 82e1d2252e760..6afed4177e1f7 100644 +--- a/llvm/test/Transforms/IndVarSimplify/pr56242.ll ++++ b/llvm/test/Transforms/IndVarSimplify/pr56242.ll +@@ -9,12 +9,14 @@ define void @test(ptr %arr) { + ; CHECK-NEXT: br label [[LOOP_HEADER:%.*]] + ; CHECK: loop.header: + ; CHECK-NEXT: [[IV:%.*]] = phi i64 [ [[IV_INC:%.*]], [[LOOP_LATCH:%.*]] ], [ 0, [[ENTRY:%.*]] ] ++; CHECK-NEXT: [[PREV:%.*]] = phi i32 [ [[V:%.*]], [[LOOP_LATCH]] ], [ 0, [[ENTRY]] ] + ; CHECK-NEXT: [[PTR:%.*]] = getelementptr inbounds i32, ptr [[ARR:%.*]], i64 [[IV]] +-; CHECK-NEXT: [[V:%.*]] = load i32, ptr [[PTR]], align 4 ++; CHECK-NEXT: [[V]] = load i32, ptr [[PTR]], align 4 + ; CHECK-NEXT: [[CMP1:%.*]] = icmp sgt i32 [[V]], 0 + ; CHECK-NEXT: br i1 [[CMP1]], label [[IF:%.*]], label [[LOOP_LATCH]] + ; CHECK: if: +-; CHECK-NEXT: call void @use(i1 false) ++; CHECK-NEXT: [[CMP2:%.*]] = icmp slt i32 [[PREV]], 0 ++; CHECK-NEXT: call void @use(i1 [[CMP2]]) + ; CHECK-NEXT: br label [[LOOP_LATCH]] + ; CHECK: loop.latch: + ; CHECK-NEXT: [[IV_INC]] = add nuw nsw i64 [[IV]], 1 diff --git a/llvm14.changes b/llvm14.changes index b85f892..03d82b9 100644 --- a/llvm14.changes +++ b/llvm14.changes @@ -1,3 +1,15 @@ +------------------------------------------------------------------- +Sat Sep 3 15:15:10 UTC 2022 - Aaron Puchert + +- Make sure we keep -DNDEBUG. At some point %{optflags} must have + lost it, perhaps because CMake usually adds it on top. So when + overriding CMAKE_{C,CXX}_FLAGS_RELWITHDEBINFO, we make sure to + take over the other flags. We drop LLVM_ENABLE_ASSERTIONS=OFF, + because that's the default anyway and hasn't helped here. +- Add llvm-scev-fix-isImpliedViaMerge.patch: fixes a miscompilation + caused by mixing up values of the current and previous iteration. + (See gh#llvm/llvm-project#56242.) + ------------------------------------------------------------------- Fri Aug 26 21:18:56 UTC 2022 - Aaron Puchert diff --git a/llvm14.spec b/llvm14.spec index 61968b1..2e77766 100644 --- a/llvm14.spec +++ b/llvm14.spec @@ -368,6 +368,8 @@ Patch24: opt-viewer-Find-style-css-in-usr-share.patch Patch25: check-no-llvm-exegesis.patch # PATCH-FIX-OPENSUSE lld-default-sha1.patch Patch26: lld-default-sha1.patch +# Fix for https://github.com/llvm/llvm-project/issues/56242. +Patch27: llvm-scev-fix-isImpliedViaMerge.patch # Fix lookup of targets in installed CMake files. (boo#1180748, https://reviews.llvm.org/D96670) Patch33: CMake-Look-up-target-subcomponents-in-LLVM_AVAILABLE_LIBS.patch # Make link dependencies of clang-repl private (https://reviews.llvm.org/D122546). @@ -801,6 +803,7 @@ This package contains the development files for Polly. %patch22 -p1 %patch24 -p1 %patch25 -p2 +%patch27 -p2 %patch33 -p2 pushd clang-%{_version}.src @@ -938,7 +941,6 @@ avail_mem=$(awk '/MemAvailable/ { print $2 }' /proc/meminfo) -DLLVM_TOOL_CLANG_TOOLS_EXTRA_BUILD:BOOL=OFF \ -DLLVM_INCLUDE_BENCHMARKS:BOOL=OFF \ -DLLVM_INCLUDE_TESTS:BOOL=OFF \ - -DLLVM_ENABLE_ASSERTIONS=OFF \ -DLLVM_TARGETS_TO_BUILD=Native \ -DCLANG_ENABLE_ARCMT:BOOL=OFF \ -DCLANG_ENABLE_STATIC_ANALYZER:BOOL=OFF \ @@ -1010,14 +1012,13 @@ export LD_LIBRARY_PATH=%{sourcedir}/build/%{_lib} %endif %endif %ifarch %arm ppc s390 %{ix86} - -DCMAKE_C_FLAGS_RELWITHDEBINFO="-g1" \ - -DCMAKE_CXX_FLAGS_RELWITHDEBINFO="-g1" \ + -DCMAKE_C_FLAGS_RELWITHDEBINFO="-O2 -g1 -DNDEBUG" \ + -DCMAKE_CXX_FLAGS_RELWITHDEBINFO="-O2 -g1 -DNDEBUG" \ %endif -DENABLE_LINKER_BUILD_ID=ON \ -DLLVM_TABLEGEN="%{sourcedir}/stage1/bin/llvm-tblgen" \ -DCLANG_TABLEGEN="%{sourcedir}/stage1/bin/clang-tblgen" \ -DLLVM_ENABLE_RTTI:BOOL=ON \ - -DLLVM_ENABLE_ASSERTIONS=OFF \ -DLLVM_ENABLE_PIC=ON \ -DLLVM_BINUTILS_INCDIR=%{_includedir} \ -DLLVM_TARGETS_TO_BUILD=%{llvm_targets} \ From e9a416c98ad2079bf067bac94c14348f20d6388c112b23fd093083ab2a7a765b Mon Sep 17 00:00:00 2001 From: Aaron Puchert Date: Sat, 3 Sep 2022 18:27:55 +0000 Subject: [PATCH 2/2] - Make llvm-scev-fix-isImpliedViaMerge.patch's test work without opaque pointers. LLVM 14 isn't using them by default yet. OBS-URL: https://build.opensuse.org/package/show/devel:tools:compiler/llvm14?expand=0&rev=39 --- llvm-scev-fix-isImpliedViaMerge.patch | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/llvm-scev-fix-isImpliedViaMerge.patch b/llvm-scev-fix-isImpliedViaMerge.patch index 7bad43b..629509f 100644 --- a/llvm-scev-fix-isImpliedViaMerge.patch +++ b/llvm-scev-fix-isImpliedViaMerge.patch @@ -19,14 +19,14 @@ index 0000000000000..82e1d2252e760 + +declare void @use(i1) + -+define void @test(ptr %arr) { ++define void @test(i32* %arr) { +; CHECK-LABEL: @test( +; CHECK-NEXT: entry: +; CHECK-NEXT: br label [[LOOP_HEADER:%.*]] +; CHECK: loop.header: +; CHECK-NEXT: [[IV:%.*]] = phi i64 [ [[IV_INC:%.*]], [[LOOP_LATCH:%.*]] ], [ 0, [[ENTRY:%.*]] ] -+; CHECK-NEXT: [[PTR:%.*]] = getelementptr inbounds i32, ptr [[ARR:%.*]], i64 [[IV]] -+; CHECK-NEXT: [[V:%.*]] = load i32, ptr [[PTR]], align 4 ++; CHECK-NEXT: [[PTR:%.*]] = getelementptr inbounds i32, i32* [[ARR:%.*]], i64 [[IV]] ++; CHECK-NEXT: [[V:%.*]] = load i32, i32* [[PTR]], align 4 +; CHECK-NEXT: [[CMP1:%.*]] = icmp sgt i32 [[V]], 0 +; CHECK-NEXT: br i1 [[CMP1]], label [[IF:%.*]], label [[LOOP_LATCH]] +; CHECK: if: @@ -45,8 +45,8 @@ index 0000000000000..82e1d2252e760 +loop.header: + %iv = phi i64 [ %iv.inc, %loop.latch ], [ 0, %entry ] + %prev = phi i32 [ %v, %loop.latch ], [ 0, %entry ] -+ %ptr = getelementptr inbounds i32, ptr %arr, i64 %iv -+ %v = load i32, ptr %ptr ++ %ptr = getelementptr inbounds i32, i32* %arr, i64 %iv ++ %v = load i32, i32* %ptr + %cmp1 = icmp sgt i32 %v, 0 + br i1 %cmp1, label %if, label %loop.latch + @@ -136,14 +136,14 @@ diff --git a/llvm/test/Transforms/IndVarSimplify/pr56242.ll b/llvm/test/Transfor index 82e1d2252e760..6afed4177e1f7 100644 --- a/llvm/test/Transforms/IndVarSimplify/pr56242.ll +++ b/llvm/test/Transforms/IndVarSimplify/pr56242.ll -@@ -9,12 +9,14 @@ define void @test(ptr %arr) { +@@ -9,12 +9,14 @@ define void @test(i32* %arr) { ; CHECK-NEXT: br label [[LOOP_HEADER:%.*]] ; CHECK: loop.header: ; CHECK-NEXT: [[IV:%.*]] = phi i64 [ [[IV_INC:%.*]], [[LOOP_LATCH:%.*]] ], [ 0, [[ENTRY:%.*]] ] +; CHECK-NEXT: [[PREV:%.*]] = phi i32 [ [[V:%.*]], [[LOOP_LATCH]] ], [ 0, [[ENTRY]] ] - ; CHECK-NEXT: [[PTR:%.*]] = getelementptr inbounds i32, ptr [[ARR:%.*]], i64 [[IV]] --; CHECK-NEXT: [[V:%.*]] = load i32, ptr [[PTR]], align 4 -+; CHECK-NEXT: [[V]] = load i32, ptr [[PTR]], align 4 + ; CHECK-NEXT: [[PTR:%.*]] = getelementptr inbounds i32, i32* [[ARR:%.*]], i64 [[IV]] +-; CHECK-NEXT: [[V:%.*]] = load i32, i32* [[PTR]], align 4 ++; CHECK-NEXT: [[V]] = load i32, i32* [[PTR]], align 4 ; CHECK-NEXT: [[CMP1:%.*]] = icmp sgt i32 [[V]], 0 ; CHECK-NEXT: br i1 [[CMP1]], label [[IF:%.*]], label [[LOOP_LATCH]] ; CHECK: if: