[PATCH] Allow InstCombiner to eliminate truncates even if it will require inserting additional instructions

[Sanjoy Das]

Overall, this LGTM given that the cleanup changes discussed happen promptly.  However, I'd wait for @majnemer to take a final look before checking in.


REPOSITORY
  rL LLVM

================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:225
@@ +224,3 @@
+    // This is not a recognized instruction. We need an explicit cast here.
+    // Currently we can reach this only when truncating expression type.
+    assert(CastInst::castIsValid(Instruction::Trunc, I, Ty) &&
----------------
It isn't clear what 'truncating expression type' means here.  Can you turn this into an assert?

================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:371
@@ -350,3 +370,3 @@
   // require duplicating the instruction in general, which isn't profitable.
   if (!I->hasOneUse()) return false;
 
----------------
Given that we're now tracking a generalized cost, does it make sense to remove this and have

```
case Inst::Add:
  bool Left = EstimateCostFor(LHS);
  bool Right = EstimateCostFor(LHS);
  if (Left && Right) {
    if (!I->hasOneUse())
      Cost++;  // new add instruction  (or may be +=2 because of code duplication)
    return true;
  } else return false;
```

(Not necessarily in this change, but in a later change?)

================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:238
@@ +237,3 @@
+    Instruction *NextI = I->getNextNode();
+    assert(NextI != I->getParent()->end() &&
+           "can not insert explicit truncate");
----------------
igor-laevsky wrote:
> sanjoy wrote:
> > I still don't understand this assert:  why can't you have IR like:
> > 
> > ```
> > %i = add i32 ...  ;; I == %i
> > br label %foo
> > 
> > foo:
> > use(%i)
> > ```
> I->getParent()->end() is end of the instruction list, not a terminator instruction. We will fail this assertion if "I" was a terminator. In other words "I++ == BB->end()"
Yes, you're right.  I confused myself.

http://reviews.llvm.org/D10571

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/