[llvm] 8d20f2c - Revert "[CodeGenPrepare] Fix isIVIncrement (PR49466)"
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 12 14:29:07 PST 2021
Jordan,
Please revert this change.
There's a couple of problems here:
* The change reverted looks obviously innocent. (e.g. it's bailing
out of a transform slightly more often)
* The change reverted was itself fixing a functional bug. At a
minimum, we'd need a larger revert to get ToT back to a sane state.
* It is generally considered reasonable to provide a test case and
wait a bit before reverting, at least once the patch is more than a
few hours old.
* Failure to provide an IR test case.
* Your test case does not reproduce. Or at least, it doesn't
reproduce when compiled with clang10 to IR and then run through
(very recent, but without your change) ToT opt -O2. If there's
something specific about the interaction of clang and opt ToT,
reducing this down to a IR test case becomes particularly important.
Philip
p.s. To be clear, I'm happy to look at your original issue this
afternoon if you've got something I can reproduce.
On 3/12/21 1:59 PM, Jordan Rupprecht via llvm-commits wrote:
> Author: Jordan Rupprecht
> Date: 2021-03-12T13:59:14-08:00
> New Revision: 8d20f2c2c66eb486ff23cc3d55a53bd840b36971
>
> URL: https://github.com/llvm/llvm-project/commit/8d20f2c2c66eb486ff23cc3d55a53bd840b36971
> DIFF: https://github.com/llvm/llvm-project/commit/8d20f2c2c66eb486ff23cc3d55a53bd840b36971.diff
>
> LOG: Revert "[CodeGenPrepare] Fix isIVIncrement (PR49466)"
>
> This reverts commit cf82700af8c658ae09b14c3d01bb1e73e48d3bd3 due to a compile timeout when building the following with `clang -O2`:
>
> ```
> template <class, class = int> class a;
> struct b {
> using d = int *;
> };
> struct e {
> using f = b::d;
> };
> class g {
> public:
> e::f h;
> e::f i;
> };
> template <class, class> class a : g {
> public:
> long j() const { return i - h; }
> long operator[](long) const noexcept;
> };
> template <class c, class k> long a<c, k>::operator[](long l) const noexcept {
> return h[l];
> }
> template <typename m, typename n> int fn1(m, n, const char *);
> int o, p;
> class D {
> void q(const a<long> &);
> long r;
> };
> void D::q(const a<long> &l) {
> int s;
> if (l[0])
> for (; l.j(); ++s) {
> if (l[s])
> while (fn1(o, 0, ""))
> ;
> r = l[s] / p;
> }
> }
> ```
>
> Added:
>
>
> Modified:
> llvm/lib/CodeGen/CodeGenPrepare.cpp
>
> Removed:
> llvm/test/CodeGen/X86/pr49466.ll
>
>
> ################################################################################
> diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
> index 0f698dd3b190..0b1156e2ace7 100644
> --- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
> +++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
> @@ -1332,7 +1332,7 @@ getIVIncrement(const PHINode *PN, const LoopInfo *LI) {
>
> static bool isIVIncrement(const BinaryOperator *BO, const LoopInfo *LI) {
> auto *PN = dyn_cast<PHINode>(BO->getOperand(0));
> - if (!PN || LI->getLoopFor(BO->getParent()) != LI->getLoopFor(PN->getParent()))
> + if (!PN)
> return false;
> if (auto IVInc = getIVIncrement(PN, LI))
> return IVInc->first == BO;
> @@ -1347,7 +1347,6 @@ bool CodeGenPrepare::replaceMathCmpWithIntrinsic(BinaryOperator *BO,
> if (!isIVIncrement(BO, LI))
> return false;
> const Loop *L = LI->getLoopFor(BO->getParent());
> - assert(L && "L should not be null after isIVIncrement()");
> // Do not risk on moving increment into a child loop.
> if (LI->getLoopFor(Cmp->getParent()) != L)
> return false;
>
> diff --git a/llvm/test/CodeGen/X86/pr49466.ll b/llvm/test/CodeGen/X86/pr49466.ll
> deleted file mode 100644
> index 4f6574d9bbf2..000000000000
> --- a/llvm/test/CodeGen/X86/pr49466.ll
> +++ /dev/null
> @@ -1,122 +0,0 @@
> -; RUN: opt < %s -O2 -codegenprepare -S | 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"
> -target triple = "x86_64-unknown-linux-gnu"
> -
> - at b = dso_local local_unnamed_addr global i64 0, align 8
> - at c = dso_local local_unnamed_addr global i64 0, align 8
> - at d = dso_local local_unnamed_addr global i64 0, align 8
> - at e = dso_local local_unnamed_addr global i64 0, align 8
> - at f = dso_local local_unnamed_addr global i64 0, align 8
> - at g = dso_local local_unnamed_addr global i64 0, align 8
> -
> -; CHECK-LABEL: @m(
> -
> -define dso_local i32 @m() local_unnamed_addr {
> -entry:
> - %0 = load i64, i64* @f, align 8
> - %1 = inttoptr i64 %0 to i32*
> - %2 = load i64, i64* @c, align 8
> - %conv18 = trunc i64 %2 to i32
> - %cmp = icmp slt i32 %conv18, 3
> - %3 = load i64, i64* @d, align 8
> - %conv43 = trunc i64 %3 to i8
> - %tobool40.not = icmp eq i8 %conv43, 0
> - br label %for.cond
> -
> -for.cond: ; preds = %for.cond39.preheader, %entry
> - %j.0 = phi i32 [ undef, %entry ], [ %j.1.lcssa, %for.cond39.preheader ]
> - %p.0 = phi i64 [ undef, %entry ], [ %p.1.lcssa, %for.cond39.preheader ]
> - %i.0 = phi i32 [ undef, %entry ], [ %i.1.lcssa, %for.cond39.preheader ]
> - %cmp73 = icmp slt i32 %i.0, 3
> - br i1 %cmp73, label %for.body.preheader, label %for.cond39.preheader
> -
> -for.body.preheader: ; preds = %for.cond
> - br label %for.body
> -
> -for.cond1.loopexit: ; preds = %for.inc34.preheader, %for.end12
> - br i1 %cmp, label %for.body, label %for.cond39.preheader.loopexit
> -
> -for.cond39.preheader.loopexit: ; preds = %for.cond1.loopexit
> - br label %for.cond39.preheader
> -
> -for.cond39.preheader: ; preds = %for.cond39.preheader.loopexit, %for.cond
> - %j.1.lcssa = phi i32 [ %j.0, %for.cond ], [ %conv18, %for.cond39.preheader.loopexit ]
> - %p.1.lcssa = phi i64 [ %p.0, %for.cond ], [ 0, %for.cond39.preheader.loopexit ]
> - %i.1.lcssa = phi i32 [ %i.0, %for.cond ], [ %conv18, %for.cond39.preheader.loopexit ]
> - br i1 %tobool40.not, label %for.cond, label %for.inc42.preheader
> -
> -for.inc42.preheader: ; preds = %for.cond39.preheader
> - br label %for.inc42
> -
> -for.body: ; preds = %for.body.preheader, %for.cond1.loopexit
> - %l.176 = phi i8 [ %sub, %for.cond1.loopexit ], [ 0, %for.body.preheader ]
> - %p.175 = phi i64 [ 0, %for.cond1.loopexit ], [ %p.0, %for.body.preheader ]
> - %j.174 = phi i32 [ %conv18, %for.cond1.loopexit ], [ %j.0, %for.body.preheader ]
> - %tobool.not = icmp eq i32 %j.174, 0
> - br i1 %tobool.not, label %cleanup45, label %for.cond2.preheader
> -
> -for.cond2.preheader: ; preds = %for.body
> - %tobool3.not69 = icmp eq i64 %p.175, 0
> - %.pr.pre = load i64, i64* @e, align 8
> - br i1 %tobool3.not69, label %for.end12, label %for.body4.preheader
> -
> -for.body4.preheader: ; preds = %for.cond2.preheader
> - %4 = sub i64 0, %p.175
> - %xtraiter = and i64 %4, 7
> - %lcmp.mod.not = icmp eq i64 %xtraiter, 0
> - br i1 %lcmp.mod.not, label %for.body4.prol.loopexit, label %for.body4.prol.preheader
> -
> -for.body4.prol.preheader: ; preds = %for.body4.preheader
> - %5 = mul nsw i64 %xtraiter, -1
> - br label %for.body4.prol
> -
> -for.body4.prol: ; preds = %for.body4.prol.preheader, %for.body4.prol
> - %lsr.iv = phi i64 [ 0, %for.body4.prol.preheader ], [ %lsr.iv.next, %for.body4.prol ]
> - %lsr.iv.next = add nsw i64 %lsr.iv, -1
> - %prol.iter.cmp.not = icmp eq i64 %5, %lsr.iv.next
> - br i1 %prol.iter.cmp.not, label %for.body4.prol.loopexit.loopexit, label %for.body4.prol
> -
> -for.body4.prol.loopexit.loopexit: ; preds = %for.body4.prol
> - %6 = sub i64 %p.175, %lsr.iv.next
> - br label %for.body4.prol.loopexit
> -
> -for.body4.prol.loopexit: ; preds = %for.body4.prol.loopexit.loopexit, %for.body4.preheader
> - %p.270.unr = phi i64 [ %p.175, %for.body4.preheader ], [ %6, %for.body4.prol.loopexit.loopexit ]
> - %7 = icmp ugt i64 %p.175, -8
> - br i1 %7, label %for.end12, label %for.body4.preheader89
> -
> -for.body4.preheader89: ; preds = %for.body4.prol.loopexit
> - br label %for.body4
> -
> -for.body4: ; preds = %for.body4.preheader89, %for.body4
> - %p.270 = phi i64 [ %inc11.7, %for.body4 ], [ %p.270.unr, %for.body4.preheader89 ]
> - %inc11.7 = add i64 %p.270, 8
> - %tobool3.not.7 = icmp eq i64 %inc11.7, 0
> - br i1 %tobool3.not.7, label %for.end12.loopexit, label %for.body4
> -
> -for.end12.loopexit: ; preds = %for.body4
> - br label %for.end12
> -
> -for.end12: ; preds = %for.end12.loopexit, %for.body4.prol.loopexit, %for.cond2.preheader
> - %8 = load i32, i32* %1, align 4
> - %conv23 = zext i32 %8 to i64
> - %9 = load i64, i64* @b, align 8
> - %div24 = udiv i64 %9, %conv23
> - store i64 %div24, i64* @b, align 8
> - %sub = add i8 %l.176, -1
> - %tobool32.not72 = icmp eq i64 %.pr.pre, 0
> - br i1 %tobool32.not72, label %for.cond1.loopexit, label %for.inc34.preheader
> -
> -for.inc34.preheader: ; preds = %for.end12
> - store i64 0, i64* @e, align 8
> - br label %for.cond1.loopexit
> -
> -for.inc42: ; preds = %for.inc42.preheader, %for.inc42
> - br label %for.inc42
> -
> -cleanup45: ; preds = %for.body
> - %cmp13 = icmp ne i8 %l.176, 0
> - %conv16 = zext i1 %cmp13 to i32
> - ret i32 %conv16
> -}
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210312/96c468af/attachment.html>
More information about the llvm-commits
mailing list