Revert "transform Y*(X+1) into X*Y+Y"

Revert "transform `Y*(X+1)` into `X*Y+Y`"

Author: pkwasnie-intel

IGC/Compiler/CISACodeGen/ShaderCodeGen.cpp

@@ -786,9 +786,6 @@ void AddLegalizationPasses(CodeGenContext &ctx, IGCPassManager &mpm, PSSignature
     // Last instruction combining pass needs to be before Legalization pass, as it can produce illegal instructions.
     mpm.add(new RemoveCodeAssumptions());
     mpm.add(createIGCInstructionCombiningPass());
-    if (ctx.platform.doIntegerMad() && ctx.m_DriverInfo.EnableIntegerMad()) {
-      mpm.add(createCanonicalizeMulAddPass());
-    }
     mpm.add(new GenSpecificPattern());
     // Cases with DPDivSqrtEmu grow significantly.
     // We can disable EarlyCSE when m_hasDPDivSqrtEmu is true,

IGC/Compiler/CustomSafeOptPass.cpp

@@ -6869,165 +6869,3 @@ bool InsertBranchOpt::runOnFunction(Function &F) {
 
   return false;
 }
-
-// Canonicalize y*(x+1) => y*x+y. This will emit mad on supported platforms.
-// This pass is dedicated to run after instcombine, which transforms x*y+y => y*(x+1).
-namespace {
-class CanonicalizeMulAdd : public FunctionPass {
-public:
-  static char ID;
-  CanonicalizeMulAdd();
-
-  StringRef getPassName() const override { return "CanonicalizeMulAdd"; }
-
-  bool runOnFunction(Function &F) override;
-
-private:
-  bool visit(BasicBlock &BB);
-  bool visitOneMad(Instruction &I);
-  bool visitTwoMads(Instruction &I);
-  bool visitRotate(Instruction &I);
-};
-} // namespace
-
-IGC_INITIALIZE_PASS_BEGIN(CanonicalizeMulAdd, "igc-canonicalize-mul-add", "Canonicalize MulAdd", false, false)
-IGC_INITIALIZE_PASS_END(CanonicalizeMulAdd, "igc-canonicalize-mul-add", "Canonicalize MulAdd", false, false)
-
-char CanonicalizeMulAdd::ID = 0;
-FunctionPass *IGC::createCanonicalizeMulAddPass() { return new CanonicalizeMulAdd(); }
-
-CanonicalizeMulAdd::CanonicalizeMulAdd() : FunctionPass(ID) {
-  initializeCanonicalizeMulAddPass(*PassRegistry::getPassRegistry());
-}
-
-bool CanonicalizeMulAdd::runOnFunction(Function &F) {
-  bool Changed = false;
-  for (auto &BB : llvm::reverse(F))
-    Changed |= visit(BB);
-  return Changed;
-}
-
-bool CanonicalizeMulAdd::visit(BasicBlock &BB) {
-
-  llvm::SmallVector<Instruction *, 8> Replaced;
-
-  for (auto II = BB.rbegin(), IE = BB.rend(); II != IE; ++II) {
-
-    if (!II->getType()->isIntegerTy() || II->getOpcode() != Instruction::Mul)
-      continue;
-
-    if (visitTwoMads(*II) || visitOneMad(*II) || visitRotate(*II))
-      Replaced.push_back(&*II);
-  }
-
-  for (auto *I : Replaced)
-    RecursivelyDeleteTriviallyDeadInstructions(I);
-
-  return !Replaced.empty();
-}
-
-// x * (y + 1) => x*y + x
-// x * (y - 1) => x*y - x
-bool CanonicalizeMulAdd::visitOneMad(Instruction &I) {
-  using namespace llvm::PatternMatch;
-
-  Value *X = nullptr, *Y = nullptr;
-  ConstantInt *CI = nullptr;
-
-  // sub1, sub2, since order of operands matters for sub
-  bool IsAdd = match(&I, m_c_Mul(m_Value(X), m_c_Add(m_Value(Y), m_ConstantInt(CI))));
-  bool IsSub1 = IsAdd ? false : match(&I, m_c_Mul(m_Value(X), m_Sub(m_Value(Y), m_ConstantInt(CI))));
-  bool IsSub2 = IsAdd || IsSub1 ? false : match(&I, m_c_Mul(m_Value(X), m_Sub(m_One(), m_Value(Y))));
-
-  if (!IsAdd && !IsSub1 && !IsSub2)
-    return false;
-
-  if (!IsSub2 && !CI->isOne() && !CI->isMinusOne())
-    return false;
-
-  auto Opcode = (IsAdd && CI->isMinusOne() || IsSub1 && CI->isOne() || IsSub2) ? Instruction::Sub : Instruction::Add;
-
-  llvm::IRBuilder<> Builder(&I);
-
-  auto *Op1 = Builder.CreateMul(X, Y);
-  auto *Op2 = X;
-
-  if (IsSub2)
-    std::swap(Op1, Op2);
-
-  I.replaceAllUsesWith(Builder.CreateBinOp(Opcode, Op1, Op2));
-  return true;
-}
-
-// Check for pattern of two consecutive mads.
-// Example:
-//   x * ((y + 1) * (z + 1))
-// Output:
-//   mad1 = x*y + x
-//   mad2 = mad1*z + mad1
-// Constant can be either 1 or -1
-bool CanonicalizeMulAdd::visitTwoMads(Instruction &I) {
-  using namespace llvm::PatternMatch;
-
-  Value *Mul = nullptr, *LHS = nullptr, *RHS = nullptr;
-  ConstantInt *LHSConst = nullptr, *RHSConst = nullptr;
-
-  if (!match(&I, m_c_Mul(m_Value(Mul), m_Mul(m_c_Add(m_Value(LHS), m_ConstantInt(LHSConst)),
-                                             m_c_Add(m_Value(RHS), m_ConstantInt(RHSConst))))))
-    return false;
-
-  if (!LHSConst->isOne() && !LHSConst->isMinusOne())
-    return false;
-
-  if (!RHSConst->isOne() && !RHSConst->isMinusOne())
-    return false;
-
-  // For consistency with instcombine, first do side not equal to Mul.
-  if (match(Mul, m_Specific(LHS))) {
-    std::swap(LHS, RHS);
-    std::swap(LHSConst, RHSConst);
-  }
-
-  auto Op1 = LHSConst->isOne() ? Instruction::Add : Instruction::Sub;
-  auto Op2 = RHSConst->isOne() ? Instruction::Add : Instruction::Sub;
-
-  llvm::IRBuilder<> Builder(&I);
-  auto *Mad1 = Builder.CreateBinOp(Op1, Builder.CreateMul(Mul, LHS), Mul);
-  auto *Mad2 = Builder.CreateBinOp(Op2, Builder.CreateMul(Mad1, RHS), Mad1);
-
-  I.replaceAllUsesWith(Mad2);
-  return true;
-}
-
-// Pattern: x * (y + z)
-// If y=x*z, then it is better to rotate instructions to: y*x + y
-// This allows to emit more mads if y=x*z is also optimized later.
-bool CanonicalizeMulAdd::visitRotate(Instruction &I) {
-  using namespace llvm::PatternMatch;
-
-  Value *Mul = nullptr, *LHS = nullptr, *RHS = nullptr;
-
-  bool IsAdd = match(&I, m_c_Mul(m_Value(Mul), m_c_Add(m_Value(LHS), m_Value(RHS))));
-  bool IsSub = IsAdd ? false : match(&I, m_c_Mul(m_Value(Mul), m_Sub(m_Value(LHS), m_Value(RHS))));
-
-  if (!IsAdd && !IsSub)
-    return false;
-
-  bool UseLHS = match(LHS, m_c_Mul(m_Specific(Mul), m_Specific(RHS)));
-  bool UseRHS = UseLHS ? false : match(RHS, m_c_Mul(m_Specific(Mul), m_Specific(LHS)));
-
-  if (!UseLHS && !UseRHS)
-    return false;
-
-  llvm::IRBuilder<> Builder(&I);
-
-  auto *Op1 = Builder.CreateMul(UseLHS ? LHS : RHS, Mul);
-  auto *Op2 = UseLHS ? LHS : RHS;
-
-  // since order of operands matters for sub
-  if (IsSub && UseRHS)
-    std::swap(Op1, Op2);
-
-  I.replaceAllUsesWith(Builder.CreateBinOp(IsAdd ? Instruction::Add : Instruction::Sub, Op1, Op2));
-  return true;
-}

IGC/Compiler/CustomSafeOptPass.hpp

@@ -220,5 +220,4 @@ llvm::FunctionPass *createCleanPHINodePass();
 llvm::FunctionPass *createMergeMemFromBranchOptPass();
 llvm::FunctionPass *createSinkLoadOptPass();
 llvm::FunctionPass *createInsertBranchOptPass();
-llvm::FunctionPass *createCanonicalizeMulAddPass();
 } // namespace IGC

IGC/Compiler/InitializePasses.h

@@ -30,7 +30,6 @@ void initializeBlockMemOpAddrScalarizationPassPass(llvm::PassRegistry &);
 void initializeBreakConstantExprPass(llvm::PassRegistry &);
 void initializeBuiltinCallGraphAnalysisPass(llvm::PassRegistry &);
 void initializeBuiltinsConverterPass(llvm::PassRegistry &);
-void initializeCanonicalizeMulAddPass(llvm::PassRegistry &);
 void initializeCastToGASAnalysisPass(llvm::PassRegistry &);
 void initializeCastToGASInfoPass(llvm::PassRegistry &);
 void initializeCastToGASInfoWrapperPass(llvm::PassRegistry &);