[CGP] Fix common type handling in optimizeMemoryInst

[skatkov]

Summary

If common type is different we should bail out due to we will not be
able to create a select or Phi of these values.

Basically it is done in ExtAddrMode::compare however it does not work
if we handle the null first and then two values of different types.
so add a check in initializeMap as well. The check in ExtAddrMode::compare
is used as earlier bail out.

[skatkov]

John, could you please review this change? We need it to try the next attempt to switch on optimization.

[john.brawn]

LGTM.

llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp

bool combineAddrModes() {
     if (AllAddrModesTrivial)
       return false;
 
     if (!addrModeCombiningAllowed())
       return false;
 
     // Build a map between <original value, basic block where we saw it> to
     // value of base register.
+    // Bail out if there is no common type.
     FoldAddrToValueMapping Map;
-    initializeMap(Map);
+    if (!initializeMap(Map))
+      return false;
 
     Value *CommonValue = findCommon(Map);
     if (CommonValue)
       AddrModes[0].SetCombinedField(DifferentField, CommonValue, AddrModes);
     return CommonValue != nullptr;
   }
 
 private:
   /// \brief Initialize Map with anchor values. For address seen in some BB
   /// we set the value of different field saw in this address.
   /// If address is not an instruction than basic block is set to null.
   /// At the same time we find a common type for different field we will
   /// use to create new Phi/Select nodes. Keep it in CommonType field.
-  void initializeMap(FoldAddrToValueMapping &Map) {
+  /// Return false if there is no common type found.
+  bool initializeMap(FoldAddrToValueMapping &Map) {
     // Keep track of keys where the value is null. We will need to replace it
     // with constant null when we know the common type.
     SmallVector<ValueInBB, 2> NullValue;
     Type *IntPtrTy = SQ.DL.getIntPtrType(AddrModes[0].OriginalValue->getType());
     for (auto &AM : AddrModes) {
       BasicBlock *BB = nullptr;
       if (Instruction *I = dyn_cast<Instruction>(AM.OriginalValue))
         BB = I->getParent();
 
       Value *DV = AM.GetFieldAsValue(DifferentField, IntPtrTy);
       if (DV) {
-        if (CommonType)
-          assert(CommonType == DV->getType() && "Different types detected!");
-        else
-          CommonType = DV->getType();
+        auto *Type = DV->getType();
+        if (CommonType && CommonType != Type)
+          return false;
+        CommonType = Type;
         Map[{ AM.OriginalValue, BB }] = DV;
       } else {
         NullValue.push_back({ AM.OriginalValue, BB });
       }
     }
     assert(CommonType && "At least one non-null value must be!");
     for (auto VIBB : NullValue)
       Map[VIBB] = Constant::getNullValue(CommonType);
+    return true;
   }
 
   /// \brief We have mapping between value A and basic block where value A
   /// seen to other value B where B was a field in addressing mode represented
   /// by A. Also we have an original value C representin an address in some
   /// basic block. Traversing from C through phi and selects we ended up with
   /// A's in a map. This utility function tries to find a value V which is a
   /// field in addressing mode C and traversing through phi nodes and selects

llvm/trunk/test/Transforms/CodeGenPrepare/X86/sink-addrmode-base.ll

; CHECK-NO-NEXT: load
   %c = phi i32* [%bc2, %if.then1], [%bc1_1, %if.then2], [%bc1_2, %if.then3]
   %v1 = load i32, i32* %c, align 4
   %g1_1 = getelementptr inbounds i64, i64* %b2, i64 5
   %bc1_1_1 = bitcast i64* %g1_1 to i32*
   %v2 = load i32, i32* %bc1_1_1, align 4
   %v = add i32 %v1, %v2
   ret i32 %v
 }
+
+; Different types but null is the first?
+define i32 @test19(i1 %cond1, i1 %cond2, i64* %b2, i8* %b1) {
+; CHECK-LABEL: @test19
+entry:
+  %g1 = getelementptr inbounds i64, i64* %b2, i64 5
+  %bc1 = bitcast i64* %g1 to i32*
+  br i1 %cond1, label %if.then1, label %if.then2
+
+if.then1:
+  %g2 = getelementptr inbounds i8, i8* %b1, i64 40
+  %bc2 = bitcast i8* %g2 to i32*
+  br label %fallthrough
+
+if.then2:
+  %bc1_1 = bitcast i64* %g1 to i32*
+  br i1 %cond2, label %fallthrough, label %if.then3
+
+if.then3:
+  %g3 = getelementptr inbounds i64, i64* null, i64 5
+  %bc1_2 = bitcast i64* %g3 to i32*
+  br label %fallthrough
+
+fallthrough:
+; CHECK-NOT: sunk_phi
+  %c = phi i32* [%bc2, %if.then1], [%bc1_1, %if.then2], [%bc1_2, %if.then3]
+  %v1 = load i32, i32* %c, align 4
+  %g1_1 = getelementptr inbounds i64, i64* %b2, i64 5
+  %bc1_1_1 = bitcast i64* %g1_1 to i32*
+  %v2 = load i32, i32* %bc1_1_1, align 4
+  %v = add i32 %v1, %v2
+  ret i32 %v
+}