Instcombine should not promote whole computation trees to "strange"

integer types, unless they are already strange.  This prevents it from
turning the code produced by SROA into crazy libcalls and stuff that
the code generator can't handle.  In the attached example, the result
was an i96 multiply that caused the x86 backend to assert.

Note that if TargetData had an idea of what the legal types are for
a target that this could be used to stop instcombine from introducing
i64 muls, as Scott wanted.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@68598 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Transforms/Scalar/InstructionCombining.cpp b/lib/Transforms/Scalar/InstructionCombining.cpp
index 4c39485b935d2b40f30c69630750b7ee98d62436..a2658b3e3f1177e8f5d31436930ef5ebf8e40c7b 100644 (file)
--- a/lib/Transforms/Scalar/InstructionCombining.cpp
+++ b/lib/Transforms/Scalar/InstructionCombining.cpp
@@ -7927,6 +7927,22 @@ Instruction *InstCombiner::commonPointerCastTransforms(CastInst &CI) {
   return commonCastTransforms(CI);
 }
 
+/// isSafeIntegerType - Return true if this is a basic integer type, not a crazy
+/// type like i42.  We don't want to introduce operations on random non-legal
+/// integer types where they don't already exist in the code.  In the future,
+/// we should consider making this based off target-data, so that 32-bit targets
+/// won't get i64 operations etc.
+static bool isSafeIntegerType(const Type *Ty) {
+  switch (Ty->getPrimitiveSizeInBits()) {
+  case 8:
+  case 16:
+  case 32:
+  case 64:
+    return true;
+  default: 
+    return false;
+  }
+}
 
 /// Only the TRUNC, ZEXT, SEXT, and BITCAST can both operand and result as
 /// integer types. This function implements the common transforms for all those
@@ -7956,6 +7972,10 @@ Instruction *InstCombiner::commonIntCastTransforms(CastInst &CI) {
   // Attempt to propagate the cast into the instruction for int->int casts.
   int NumCastsRemoved = 0;
   if (!isa<BitCastInst>(CI) &&
+      // Only do this if the dest type is a simple type, don't convert the
+      // expression tree to something weird like i93 unless the source is also
+      // strange.
+      (isSafeIntegerType(DestTy) || !isSafeIntegerType(SrcI->getType())) &&
       CanEvaluateInDifferentType(SrcI, cast<IntegerType>(DestTy),
                                  CI.getOpcode(), NumCastsRemoved)) {
     // If this cast is a truncate, evaluting in a different type always

diff --git a/test/Transforms/InstCombine/2009-04-07-MulPromoteToI96.ll b/test/Transforms/InstCombine/2009-04-07-MulPromoteToI96.ll
new file mode 100644 (file)
index 0000000..82f8762
--- /dev/null
+++ b/test/Transforms/InstCombine/2009-04-07-MulPromoteToI96.ll
@@ -0,0 +1,13 @@
+; RUN: llvm-as < %s | opt -instcombine | llvm-dis | grep {mul i64}
+; rdar://6762288
+
+; Instcombine should not promote the mul to i96 because it is definitely
+; not a legal type for the target, and we don't want a libcall.
+
+define i96 @test(i96 %a.4, i96 %b.2) {
+       %tmp1086 = trunc i96 %a.4 to i64                ; <i64> [#uses=1]
+       %tmp836 = trunc i96 %b.2 to i64         ; <i64> [#uses=1]
+       %mul185 = mul i64 %tmp1086, %tmp836             ; <i64> [#uses=1]
+       %tmp544 = zext i64 %mul185 to i96               ; <i96> [#uses=1]
+       ret i96 %tmp544
+}