[CodeGen] Error on unsupported checked multiplies early

[vsk]

Summary

LLVM's smul.with.overflow intrinsic isn't supported on X86 for bit
widths larger than 64, or on X86-64 for bit widths larger than 128.

The failure mode is either a linker error ("the __muloti4 builtin isn't
available for this target") or an assertion failure ("SelectionDAG
doesn't know what builtin to call").

Until we actually add builtin support for 128-bit multiply-with-overflow
on X86, we should error-out on unsupported calls as early as possible.

https://bugs.llvm.org/show_bug.cgi?id=34920

[rjmccall]

lib/CodeGen/CGBuiltin.cpp

2272

Is there a reason this only fails on x86? If LLVM doesn't have generic wide-operation lowering, this probably needs to be a target whitelist rather than a blacklist.

[vsk]

• Update to check against a whitelist of supported targets.

[vsk]

lib/CodeGen/CGBuiltin.cpp

2272

That makes sense. For the 128-bit operation, the whitelist is {x86-64, wasm64, mips64}. We don't support this operation for bit widths larger than 128 bits on any target. I'll update the patch accordingly.

[rjmccall]

Okay. Sounds good to me.

We intend to actually implement the generic lowering, I hope?

[vsk]

In D38861#896435, @rjmccall wrote:

Okay. Sounds good to me.

We intend to actually implement the generic lowering, I hope?

Yes, I'll make a note of that in PR34920, and am tracking the bug internally in rdar://problem/34963321.

[joerg]

lib/CodeGen/CGBuiltin.cpp

2272

That sounds wrong. __int128_t should be supported by all 64bit architectures, not just those three.

[vsk]

lib/CodeGen/CGBuiltin.cpp

2272

I didn't mean to imply generic int128_t operations aren't supported. This patch just focuses on muloti4 (signed multiplication with overflow checking).

[vsk]

Abandoned in favor of a proper fix: https://reviews.llvm.org/D41149

lib/CodeGen/CGBuiltin.cpp

for (const auto &Type : Types) {
     if (Width < MinWidth) {
       Width = MinWidth;
     }
   }
 
   return {Width, Signed};
 }
 
+// Check if the target supports checked multiplication with 128-bit operands.
+static bool has128BitMulOverflowSupport(const llvm::Triple &Triple) {
+  if (!Triple.isArch64Bit())
+    return false;
+  return StringSwitch<bool>(llvm::Triple::getArchTypePrefix(Triple.getArch()))
+      .Cases("x86", "wasm", "mips", true)
+      .Default(false);
+}
+
 Value *CodeGenFunction::EmitVAStartEnd(Value *ArgValue, bool IsStart) {
   llvm::Type *DestType = Int8PtrTy;
   if (ArgValue->getType() != DestType)
     ArgValue =
         Builder.CreateBitCast(ArgValue, DestType, ArgValue->getName().data());
 
   Intrinsic::ID inst = IsStart ? Intrinsic::vastart : Intrinsic::vaend;
   return Builder.CreateCall(CGM.getIntrinsic(inst), ArgValue);
WidthAndSignedness LeftInfo =
         getIntegerWidthAndSignedness(CGM.getContext(), LeftArg->getType());
     WidthAndSignedness RightInfo =
         getIntegerWidthAndSignedness(CGM.getContext(), RightArg->getType());
     WidthAndSignedness ResultInfo =
         getIntegerWidthAndSignedness(CGM.getContext(), ResultQTy);
     WidthAndSignedness EncompassingInfo =
         EncompassingIntegerType({LeftInfo, RightInfo, ResultInfo});
 
+    llvm::Type *ResultLLVMTy = CGM.getTypes().ConvertType(ResultQTy);
+
+    if (BuiltinID == Builtin::BI__builtin_mul_overflow) {
+      if ((EncompassingInfo.Width > 64 &&
+           !has128BitMulOverflowSupport(getTarget().getTriple())) ||
+          (EncompassingInfo.Width > 128)) {
+        CGM.ErrorUnsupported(E,
+                             "__builtin_mul_overflow with mixed-sign operands");
+        return RValue::get(llvm::UndefValue::get(ResultLLVMTy));
+      }
+    }
+
     llvm::Type *EncompassingLLVMTy =
         llvm::IntegerType::get(CGM.getLLVMContext(), EncompassingInfo.Width);
 
-    llvm::Type *ResultLLVMTy = CGM.getTypes().ConvertType(ResultQTy);
-
     llvm::Intrinsic::ID IntrinsicId;
     switch (BuiltinID) {
     default:
       llvm_unreachable("Unknown overflow builtin id.");
     case Builtin::BI__builtin_add_overflow:
       IntrinsicId = EncompassingInfo.Signed
                         ? llvm::Intrinsic::sadd_with_overflow
                         : llvm::Intrinsic::uadd_with_overflow;

test/CodeGen/builtins-overflow-unsupported.c

+// RUN: not %clang_cc1 -triple "i686-unknown-unknown"   -emit-llvm -x c %s -o - 2>&1 | FileCheck %s --check-prefix=M32
+// RUN: not %clang_cc1 -triple "x86_64-unknown-unknown" -emit-llvm -x c %s -o - 2>&1 | FileCheck %s --check-prefix=M64
+
+signed long long try_smul_i65(signed long long a, unsigned long long b) {
+  // M32: [[@LINE+1]]:10: error: cannot compile this __builtin_mul_overflow with mixed-sign operands yet
+  return __builtin_mul_overflow(a, b, &b);
+}
+
+#if defined(__LP64__)
+__int128_t try_smul_i29(__int128_t a, __uint128_t b) {
+  // M64: [[@LINE+1]]:10: error: cannot compile this __builtin_mul_overflow with mixed-sign operands yet
+  return __builtin_mul_overflow(a, b, &b);
+}
+#endif

test/CodeGen/builtins-overflow.c

 // Test CodeGen for Security Check Overflow Builtins.
 // rdar://13421498
 
 // RUN: %clang_cc1 -triple "i686-unknown-unknown"   -emit-llvm -x c %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple "x86_64-unknown-unknown" -emit-llvm -x c %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple "x86_64-unknown-unknown" -emit-llvm -x c %s -o - | FileCheck %s --check-prefixes=CHECK,M64
+// RUN: %clang_cc1 -triple "wasm64-unknown-unknown" -emit-llvm -x c %s -o - | FileCheck %s --check-prefixes=M64
+// RUN: %clang_cc1 -triple "mips64-unknown-unknown" -emit-llvm -x c %s -o - | FileCheck %s --check-prefixes=M64
 // RUN: %clang_cc1 -triple "x86_64-mingw32"         -emit-llvm -x c %s -o - | FileCheck %s
 
 extern unsigned UnsignedErrorCode;
 extern unsigned long UnsignedLongErrorCode;
 extern unsigned long long UnsignedLongLongErrorCode;
 extern int IntErrorCode;
 extern long LongErrorCode;
 extern long long LongLongErrorCode;

 long long test_smulll_overflow(long long x, long long y) {
 // CHECK: @test_smulll_overflow
 // CHECK: %{{.+}} = call { i64, i1 } @llvm.smul.with.overflow.i64(i64 %{{.+}}, i64 %{{.+}})
   long long result;
   if (__builtin_smulll_overflow(x, y, &result))
     return LongLongErrorCode;
   return result;
 }
+
+#if defined(__LP64__)
+signed long long test_mixed_sign_mul_i64(signed long long a, unsigned long long b) {
+  // M64-LABEL: define i64 @test_mixed_sign_mul_i64
+  // M64: sext i64 {{.*}} to i65
+  // M64-NEXT: zext i64 {{.*}} to i65
+  // M64-NEXT: call { i65, i1 } @llvm.smul.with.overflow.i65
+  // M64-NEXT: [[OFLOW_1:%.*]] = extractvalue { i65, i1 } {{.*}}, 1
+  // M64-NEXT: [[RES:%.*]] = extractvalue { i65, i1 } {{.*}}, 0
+  // M64-NEXT: [[RES_TRUNC:%.*]] = trunc i65 {{.*}} to i64
+  // M64-NEXT: [[RES_EXT:%.*]] = zext i64 {{.*}} to i65
+  // M64-NEXT: [[OFLOW_2:%.*]] = icmp ne i65 [[RES]], [[RES_EXT]]
+  // M64-NEXT: or i1 [[OFLOW_1]], [[OFLOW_2]]
+  // M64-NEXT: store i64 [[RES_TRUNC]]
+  return __builtin_mul_overflow(a, b, &b);
+}
+#endif