From cc58a593a2b975bd46a25749287872d0d6978488 Mon Sep 17 00:00:00 2001 From: Erik Verbruggen Date: Fri, 28 Mar 2014 14:42:34 +0000 Subject: Revert "GVN: merge overflow intrinsics with non-overflow instructions." This reverts commit r203553, and follow-up commits r203558 and r203574. I will follow this up on the mailinglist to do it in a way that won't cause subtle PRE bugs. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@205009 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/Scalar/GVN.cpp | 182 +++++++++++++--------------------------- test/Transforms/GVN/overflow.ll | 67 --------------- 2 files changed, 58 insertions(+), 191 deletions(-) delete mode 100644 test/Transforms/GVN/overflow.ll diff --git a/lib/Transforms/Scalar/GVN.cpp b/lib/Transforms/Scalar/GVN.cpp index 53c120530f..33c387cb71 100644 --- a/lib/Transforms/Scalar/GVN.cpp +++ b/lib/Transforms/Scalar/GVN.cpp @@ -111,11 +111,10 @@ namespace { uint32_t nextValueNumber; Expression create_expression(Instruction* I); - Expression create_intrinsic_expression(CallInst *C, uint32_t opcode, - bool IsCommutative); Expression create_cmp_expression(unsigned Opcode, CmpInst::Predicate Predicate, Value *LHS, Value *RHS); + Expression create_extractvalue_expression(ExtractValueInst* EI); uint32_t lookup_or_add_call(CallInst* C); public: ValueTable() : nextValueNumber(1) { } @@ -189,33 +188,6 @@ Expression ValueTable::create_expression(Instruction *I) { for (InsertValueInst::idx_iterator II = E->idx_begin(), IE = E->idx_end(); II != IE; ++II) e.varargs.push_back(*II); - } else if (ExtractValueInst *EVI = dyn_cast(I)) { - for (ExtractValueInst::idx_iterator II = EVI->idx_begin(), - IE = EVI->idx_end(); II != IE; ++II) - e.varargs.push_back(*II); - } - - return e; -} - -Expression ValueTable::create_intrinsic_expression(CallInst *C, uint32_t opcode, - bool IsCommutative) { - Expression e; - e.opcode = opcode; - StructType *ST = cast(C->getType()); - assert(ST); - e.type = *ST->element_begin(); - - for (unsigned i = 0, ei = C->getNumArgOperands(); i < ei; ++i) - e.varargs.push_back(lookup_or_add(C->getArgOperand(i))); - if (IsCommutative) { - // Ensure that commutative instructions that only differ by a permutation - // of their operands get the same value number by sorting the operand value - // numbers. Since all commutative instructions have two operands it is more - // efficient to sort by hand rather than using, say, std::sort. - assert(C->getNumArgOperands() == 2 && "Unsupported commutative instruction!"); - if (e.varargs[0] > e.varargs[1]) - std::swap(e.varargs[0], e.varargs[1]); } return e; @@ -240,6 +212,58 @@ Expression ValueTable::create_cmp_expression(unsigned Opcode, return e; } +Expression ValueTable::create_extractvalue_expression(ExtractValueInst *EI) { + assert(EI != 0 && "Not an ExtractValueInst?"); + Expression e; + e.type = EI->getType(); + e.opcode = 0; + + IntrinsicInst *I = dyn_cast(EI->getAggregateOperand()); + if (I != 0 && EI->getNumIndices() == 1 && *EI->idx_begin() == 0 ) { + // EI might be an extract from one of our recognised intrinsics. If it + // is we'll synthesize a semantically equivalent expression instead on + // an extract value expression. + switch (I->getIntrinsicID()) { + case Intrinsic::sadd_with_overflow: + case Intrinsic::uadd_with_overflow: + e.opcode = Instruction::Add; + break; + case Intrinsic::ssub_with_overflow: + case Intrinsic::usub_with_overflow: + e.opcode = Instruction::Sub; + break; + case Intrinsic::smul_with_overflow: + case Intrinsic::umul_with_overflow: + e.opcode = Instruction::Mul; + break; + default: + break; + } + + if (e.opcode != 0) { + // Intrinsic recognized. Grab its args to finish building the expression. + assert(I->getNumArgOperands() == 2 && + "Expect two args for recognised intrinsics."); + e.varargs.push_back(lookup_or_add(I->getArgOperand(0))); + e.varargs.push_back(lookup_or_add(I->getArgOperand(1))); + return e; + } + } + + // Not a recognised intrinsic. Fall back to producing an extract value + // expression. + e.opcode = EI->getOpcode(); + for (Instruction::op_iterator OI = EI->op_begin(), OE = EI->op_end(); + OI != OE; ++OI) + e.varargs.push_back(lookup_or_add(*OI)); + + for (ExtractValueInst::idx_iterator II = EI->idx_begin(), IE = EI->idx_end(); + II != IE; ++II) + e.varargs.push_back(*II); + + return e; +} + //===----------------------------------------------------------------------===// // ValueTable External Functions //===----------------------------------------------------------------------===// @@ -373,29 +397,8 @@ uint32_t ValueTable::lookup_or_add(Value *V) { Instruction* I = cast(V); Expression exp; switch (I->getOpcode()) { - case Instruction::Call: { - CallInst *C = cast(I); - if (IntrinsicInst *II = dyn_cast(C)) { - switch (II->getIntrinsicID()) { - case Intrinsic::sadd_with_overflow: - case Intrinsic::uadd_with_overflow: - exp = create_intrinsic_expression(C, Instruction::Add, true); - break; - case Intrinsic::ssub_with_overflow: - case Intrinsic::usub_with_overflow: - exp = create_intrinsic_expression(C, Instruction::Sub, false); - break; - case Intrinsic::smul_with_overflow: - case Intrinsic::umul_with_overflow: - exp = create_intrinsic_expression(C, Instruction::Mul, true); - break; - default: - return lookup_or_add_call(C); - } - } else { - return lookup_or_add_call(C); - } - } break; + case Instruction::Call: + return lookup_or_add_call(cast(I)); case Instruction::Add: case Instruction::FAdd: case Instruction::Sub: @@ -434,9 +437,11 @@ uint32_t ValueTable::lookup_or_add(Value *V) { case Instruction::ShuffleVector: case Instruction::InsertValue: case Instruction::GetElementPtr: - case Instruction::ExtractValue: exp = create_expression(I); break; + case Instruction::ExtractValue: + exp = create_extractvalue_expression(cast(I)); + break; default: valueNumbering[V] = nextValueNumber; return nextValueNumber++; @@ -2184,54 +2189,6 @@ bool GVN::propagateEquality(Value *LHS, Value *RHS, return Changed; } -static bool normalOpAfterIntrinsic(Instruction *I, Value *Repl) -{ - switch (I->getOpcode()) { - case Instruction::Add: - if (IntrinsicInst *II = dyn_cast(Repl)) - return II->getIntrinsicID() == Intrinsic::sadd_with_overflow - || II->getIntrinsicID() == Intrinsic::uadd_with_overflow; - return false; - case Instruction::Sub: - if (IntrinsicInst *II = dyn_cast(Repl)) - return II->getIntrinsicID() == Intrinsic::ssub_with_overflow - || II->getIntrinsicID() == Intrinsic::usub_with_overflow; - return false; - case Instruction::Mul: - if (IntrinsicInst *II = dyn_cast(Repl)) - return II->getIntrinsicID() == Intrinsic::smul_with_overflow - || II->getIntrinsicID() == Intrinsic::umul_with_overflow; - return false; - default: - return false; - } -} - -static bool intrinsicAterNormalOp(Instruction *I, Value *Repl) -{ - IntrinsicInst *II = dyn_cast(I); - if (!II) - return false; - - Instruction *RI = dyn_cast(Repl); - if (!RI) - return false; - - switch (RI->getOpcode()) { - case Instruction::Add: - return II->getIntrinsicID() == Intrinsic::sadd_with_overflow - || II->getIntrinsicID() == Intrinsic::uadd_with_overflow; - case Instruction::Sub: - return II->getIntrinsicID() == Intrinsic::ssub_with_overflow - || II->getIntrinsicID() == Intrinsic::usub_with_overflow; - case Instruction::Mul: - return II->getIntrinsicID() == Intrinsic::smul_with_overflow - || II->getIntrinsicID() == Intrinsic::umul_with_overflow; - default: - return false; - } -} - /// processInstruction - When calculating availability, handle an instruction /// by inserting it into the appropriate sets bool GVN::processInstruction(Instruction *I) { @@ -2345,27 +2302,6 @@ bool GVN::processInstruction(Instruction *I) { return false; } - if (normalOpAfterIntrinsic(I, repl)) { - // An intrinsic followed by a normal operation (e.g. sadd_with_overflow - // followed by a sadd): replace the second instruction with an extract. - IntrinsicInst *II = cast(repl); - assert(II); - repl = ExtractValueInst::Create(II, 0, I->getName() + ".repl", I); - } else if (intrinsicAterNormalOp(I, repl)) { - // A normal operation followed by an intrinsic (e.g. sadd followed by a - // sadd_with_overflow). - // Clone the intrinsic, and insert it before the replacing instruction. Then - // replace the (current) instruction with the cloned one. In a subsequent - // run, the original replacement (the non-intrinsic) will be be replaced by - // the new intrinsic. - Instruction *RI = dyn_cast(repl); - assert(RI); - Instruction *newIntrinsic = I->clone(); - newIntrinsic->setName(I->getName() + ".repl"); - newIntrinsic->insertBefore(RI); - repl = newIntrinsic; - } - // Remove it! patchAndReplaceAllUsesWith(I, repl); if (MD && repl->getType()->getScalarType()->isPointerTy()) @@ -2550,8 +2486,6 @@ bool GVN::performPRE(Function &F) { predMap.push_back(std::make_pair(static_cast(0), P)); PREPred = P; ++NumWithout; - } else if (predV->getType() != CurInst->getType()) { - continue; } else if (predV == CurInst) { /* CurInst dominates this predecessor. */ NumWithout = 2; diff --git a/test/Transforms/GVN/overflow.ll b/test/Transforms/GVN/overflow.ll deleted file mode 100644 index 8c00573bea..0000000000 --- a/test/Transforms/GVN/overflow.ll +++ /dev/null @@ -1,67 +0,0 @@ -; RUN: opt -S -gvn < %s | FileCheck %s - -define i32 @sadd1(i32 %a, i32 %b) #0 { -; CHECK-LABEL: @sadd1( -entry: - %sadd = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 %a, i32 %b) - %cmp = extractvalue { i32, i1 } %sadd, 1 - br i1 %cmp, label %if.then, label %if.end - -if.then: ; preds = %entry - ret i32 42 - -if.end: ; preds = %entry - %sadd3 = add i32 %a, %b - ret i32 %sadd3 -; CHECK-NOT: add i32 %a, %b -; CHECK: %sadd3.repl = extractvalue { i32, i1 } %sadd, 0 -; CHECK: ret i32 %sadd3.repl -} - -define i32 @sadd2(i32 %a, i32 %b) #0 { -entry: - %sadd3 = add i32 %a, %b - %sadd = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 %a, i32 %b) - %cmp = extractvalue { i32, i1 } %sadd, 1 - br i1 %cmp, label %if.then, label %if.end -; CHECK-NOT: %sadd3 = add i32 %a, %b -; CHECK: %sadd.repl = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 %a, i32 %b) -; CHECK-NOT: %sadd = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 %a, i32 %b) -; CHECK: %sadd3.repl = extractvalue { i32, i1 } %sadd.repl, 0 - -if.then: ; preds = %entry - %sadd4 = add i32 %sadd3, 1 - ret i32 %sadd4 -; CHECK: %sadd4 = add i32 %sadd3.repl, 1 - -if.end: ; preds = %entry - ret i32 %sadd3 -; CHECK: ret i32 %sadd3.repl -} - -; Check if PRE does not crash -define i32 @pre(i32 %a, i32 %b) nounwind ssp uwtable { -entry: - %cmp = icmp sgt i32 %a, 42 - br i1 %cmp, label %if.then, label %if.end3 - -if.then: ; preds = %entry - %add = call {i32, i1} @llvm.sadd.with.overflow.i32(i32 %a, i32 %b) - %add1 = extractvalue {i32, i1} %add, 0 - %o = extractvalue {i32, i1} %add, 1 - %o32 = zext i1 %o to i32 - %add32 = add i32 %add1, %o32 - %cmp1 = icmp sgt i32 %add1, 42 - br i1 %cmp1, label %if.then2, label %if.end3 - -if.then2: ; preds = %if.then - call void @abort() noreturn - unreachable - -if.end3: ; preds = %if.end, %entry - %add4 = add i32 %a, %b - ret i32 %add4 -} - -declare void @abort() noreturn -declare { i32, i1 } @llvm.sadd.with.overflow.i32(i32, i32) nounwind readnone -- cgit v1.2.3