Date: Wed, 18 Mar 2020 20:44:40 +0000 (UTC) From: Dimitry Andric <dim@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r359086 - head/contrib/llvm-project/llvm/lib/Transforms/Scalar Message-ID: <202003182044.02IKieL7099726@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: dim Date: Wed Mar 18 20:44:40 2020 New Revision: 359086 URL: https://svnweb.freebsd.org/changeset/base/359086 Log: Merge commit b8ebc11f0 from llvm git (by Sanjay Patel): [EarlyCSE] avoid crashing when detecting min/max/abs patterns (PR41083) As discussed in PR41083: https://bugs.llvm.org/show_bug.cgi?id=41083 ...we can assert/crash in EarlyCSE using the current hashing scheme and instructions with flags. ValueTracking's matchSelectPattern() may rely on overflow (nsw, etc) or other flags when detecting patterns such as min/max/abs composed of compare+select. But the value numbering / hashing mechanism used by EarlyCSE intersects those flags to allow more CSE. Several alternatives to solve this are discussed in the bug report. This patch avoids the issue by doing simple matching of min/max/abs patterns that never requires instruction flags. We give up some CSE power because of that, but that is not expected to result in much actual performance difference because InstCombine will canonicalize these patterns when possible. It even has this comment for abs/nabs: /// Canonicalize all these variants to 1 pattern. /// This makes CSE more likely. (And this patch adds PhaseOrdering tests to verify that the expected transforms are still happening in the standard optimization pipelines. I left this code to use ValueTracking's "flavor" enum values, so we don't have to change the callers' code. If we decide to go back to using the ValueTracking call (by changing the hashing algorithm instead), it should be obvious how to replace this chunk. Differential Revision: https://reviews.llvm.org/D74285 This fixes an assertion when building the math/gsl port on PowerPC64. Requested by: pkubja MFC after: 6 weeks X-MFC-With: 358851 Modified: head/contrib/llvm-project/llvm/lib/Transforms/Scalar/EarlyCSE.cpp Modified: head/contrib/llvm-project/llvm/lib/Transforms/Scalar/EarlyCSE.cpp ============================================================================== --- head/contrib/llvm-project/llvm/lib/Transforms/Scalar/EarlyCSE.cpp Wed Mar 18 20:38:15 2020 (r359085) +++ head/contrib/llvm-project/llvm/lib/Transforms/Scalar/EarlyCSE.cpp Wed Mar 18 20:44:40 2020 (r359086) @@ -152,13 +152,50 @@ static bool matchSelectWithOptionalNotCond(Value *V, V std::swap(A, B); } - // Set flavor if we find a match, or set it to unknown otherwise; in - // either case, return true to indicate that this is a select we can - // process. - if (auto *CmpI = dyn_cast<ICmpInst>(Cond)) - Flavor = matchDecomposedSelectPattern(CmpI, A, B, A, B).Flavor; - else - Flavor = SPF_UNKNOWN; + // Match canonical forms of abs/nabs/min/max. We are not using ValueTracking's + // more powerful matchSelectPattern() because it may rely on instruction flags + // such as "nsw". That would be incompatible with the current hashing + // mechanism that may remove flags to increase the likelihood of CSE. + + // These are the canonical forms of abs(X) and nabs(X) created by instcombine: + // %N = sub i32 0, %X + // %C = icmp slt i32 %X, 0 + // %ABS = select i1 %C, i32 %N, i32 %X + // + // %N = sub i32 0, %X + // %C = icmp slt i32 %X, 0 + // %NABS = select i1 %C, i32 %X, i32 %N + Flavor = SPF_UNKNOWN; + CmpInst::Predicate Pred; + if (match(Cond, m_ICmp(Pred, m_Specific(B), m_ZeroInt())) && + Pred == ICmpInst::ICMP_SLT && match(A, m_Neg(m_Specific(B)))) { + // ABS: B < 0 ? -B : B + Flavor = SPF_ABS; + return true; + } + if (match(Cond, m_ICmp(Pred, m_Specific(A), m_ZeroInt())) && + Pred == ICmpInst::ICMP_SLT && match(B, m_Neg(m_Specific(A)))) { + // NABS: A < 0 ? A : -A + Flavor = SPF_NABS; + return true; + } + + if (!match(Cond, m_ICmp(Pred, m_Specific(A), m_Specific(B)))) { + // Check for commuted variants of min/max by swapping predicate. + // If we do not match the standard or commuted patterns, this is not a + // recognized form of min/max, but it is still a select, so return true. + if (!match(Cond, m_ICmp(Pred, m_Specific(B), m_Specific(A)))) + return true; + Pred = ICmpInst::getSwappedPredicate(Pred); + } + + switch (Pred) { + case CmpInst::ICMP_UGT: Flavor = SPF_UMAX; break; + case CmpInst::ICMP_ULT: Flavor = SPF_UMIN; break; + case CmpInst::ICMP_SGT: Flavor = SPF_SMAX; break; + case CmpInst::ICMP_SLT: Flavor = SPF_SMIN; break; + default: break; + } return true; }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202003182044.02IKieL7099726>