From e98ec77a95a9045e47d13c9f5f0df7d912997026 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Thu, 15 Nov 2018 14:53:37 +0000 Subject: [InstSimplify] delete shift-of-zero guard ops around funnel shifts This is a problem seen in common rotate idioms as noted in: https://bugs.llvm.org/show_bug.cgi?id=34924 Note that we are not canonicalizing standard IR (shifts and logic) to the intrinsics yet. (Although I've written this before...) I think this is the last step before we enable that transform. Ie, we could regress code by doing that transform without this simplification in place. In PR34924, I questioned whether this is a valid transform for target-independent IR, but I convinced myself this is ok. If we're speculating a funnel shift by turning cmp+br into select, then SimplifyCFG has already determined that the transform is justified. It's possible that SimplifyCFG is not taking into account profile or other metadata, but if that's true, then it's a bug independent of funnel shifts. Also, we do have CGP code to restore a guard like this around an intrinsic if it can't be lowered cheaply. But that isn't necessary for funnel shift because the default expansion in SelectionDAGBuilder includes this same cmp+select. Differential Revision: https://reviews.llvm.org/D54552 llvm-svn: 346960 --- llvm/lib/Analysis/InstructionSimplify.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'llvm/lib/Analysis/InstructionSimplify.cpp') diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp index fd6f4ba476e..0039d1eebc0 100644 --- a/llvm/lib/Analysis/InstructionSimplify.cpp +++ b/llvm/lib/Analysis/InstructionSimplify.cpp @@ -3837,6 +3837,28 @@ static Value *simplifySelectWithICmpCond(Value *CondVal, Value *TrueVal, if (Value *V = simplifySelectBitTest(TrueVal, FalseVal, X, Y, Pred == ICmpInst::ICMP_EQ)) return V; + + // Test for zero-shift-guard-ops around funnel shifts. These are used to + // avoid UB from oversized shifts in raw IR rotate patterns, but the + // intrinsics do not have that problem. + Value *ShAmt; + auto isFsh = m_CombineOr(m_Intrinsic(m_Value(X), m_Value(), + m_Value(ShAmt)), + m_Intrinsic(m_Value(), m_Value(X), + m_Value(ShAmt))); + // (ShAmt != 0) ? fshl(X, *, ShAmt) : X --> fshl(X, *, ShAmt) + // (ShAmt != 0) ? fshr(*, X, ShAmt) : X --> fshr(*, X, ShAmt) + // (ShAmt == 0) ? fshl(X, *, ShAmt) : X --> X + // (ShAmt == 0) ? fshr(*, X, ShAmt) : X --> X + if (match(TrueVal, isFsh) && FalseVal == X && CmpLHS == ShAmt) + return Pred == ICmpInst::ICMP_NE ? TrueVal : X; + + // (ShAmt == 0) ? X : fshl(X, *, ShAmt) --> fshl(X, *, ShAmt) + // (ShAmt == 0) ? X : fshr(*, X, ShAmt) --> fshr(*, X, ShAmt) + // (ShAmt != 0) ? X : fshl(X, *, ShAmt) --> X + // (ShAmt != 0) ? X : fshr(*, X, ShAmt) --> X + if (match(FalseVal, isFsh) && TrueVal == X && CmpLHS == ShAmt) + return Pred == ICmpInst::ICMP_EQ ? FalseVal : X; } // Check for other compares that behave like bit test. -- cgit v1.2.3