Fix size for _ExtInt types with builtins

[jtmott-intel]

Summary

This patch addresses two issues.

The first issue is use of the checked arithmetic builtins in combination with _ExtInt types. _ExtInt types with non-power of two sizes would be (inconsistently) rounded up to the next largest power of two. The inconsistency caused the following compile error:

$ cat test.c
int main(void) {
    _ExtInt(31) x = 1;
    _ExtInt(31) y = 1;
    _ExtInt(31) result;
    _Bool status = __builtin_add_overflow(x, y, &result);
    return 0;
}

$ clang test.c
clang-11: /home/user/llvm_workspace/llvm-project/llvm/lib/IR/Instructions.cpp:1408: void llvm::StoreInst::AssertOK(): Assertion `getOperand(0)->getType() == cast<PointerType>(getOperand(1)->getType())->getElementType() && "Ptr must be a pointer to Val type!"' failed.

To fix this, I added the above example as a failing test to clang/test/CodeGen/builtins-overflow.c, and I changed clang/lib/CodeGen/CGBuiltin.cpp to get the correct size for _ExtInt types and allow the new test to pass.

The second issue is with __builtin_mul_overflow specifically in combination with _ExtInt types larger than 64 bits. For that combination, the emitted IR appears to be correct, but a full compile caused the following link error (for at least X86 target):

$ cat test.c
int main(void) {
    _ExtInt(65) x = 1;
    _ExtInt(65) y = 1;
    _ExtInt(65) result;
    _Bool status = __builtin_mul_overflow(x, y, &result);
    return 0;
}
$ clang test.c
/tmp/test-2b8174.o: In function `main':
test.c:(.text+0x63): undefined reference to `__muloti4'
clang-11: error: linker command failed with exit code 1 (use -v to see invocation)

To address this -- for now -- I changed clang/lib/Sema/SemaChecking.cpp to emit a diagnostic that explains the limitation to users better than the link error does. I did my best to borrow verbiage from similar messages. The new compile result is:

$ clang test.c
test.c:5:43: error: _ExtInt argument larger than 64-bits to overflow builtin requires runtime support that is not available for this target
    _Bool status = __builtin_mul_overflow(x, y, &result);
                                          ^
1 error generated.

[erichkeane]

So the idea of the SEMA check is right, but it ultimately isn't correct. Integers up to 128 can be called, as long as you use compiler-rt (--rtlib=compiler-rt), see https://godbolt.org/z/JDXZG_.

However, 129 bits results in a crash in LLVM: https://godbolt.org/z/q432t_

When you change this, please add tests for 128/127 bit as well. AND the new error message needs a SEMA test as well.

clang/include/clang/Basic/DiagnosticSemaKinds.td

7951

Mentioning target-specific support here seems incorrect. @rjmccall I cannot think of a better wording, can you?

clang/lib/Sema/SemaChecking.cpp

334

I'll explain it below, but while I think this is the right IDEA, I think >64 is wrong. It should be >128.

Second, this requires a SEMA test.

[erichkeane]

Also, please file a bug (if you haven't already) about the >128 bit problem. We have a similar bug for normal division (https://bugs.llvm.org/show_bug.cgi?id=45649), but this needs one as well.

[lebedev.ri]

clang/lib/Sema/SemaChecking.cpp

1740–1741

I don't think this applies to add/sub: https://godbolt.org/z/Dj3GLP (or at least not all of them?)
OTOH this does apply to plain division/remainder: https://godbolt.org/z/SXNw8H

[rjmccall]

Linking compiler-rt is something that should be automatically happening. It's possible that compiler-rt will have different maximum widths on different targets, though.

clang/include/clang/Basic/DiagnosticSemaKinds.td

7951

"overflow builtins do not support _ExtInt operands of more than %0 bits on this target"? I don't think it's unreasonable to mention the target-specificness of it. Hard-coding the number 64 in the diagnostic seems excessively targeted, though.

[jtmott-intel]

Updated the diagnostic check and message for 128 rather than 64 bits.
Added sema tests for 128/129 sizes.
Added codegen tests for 127/128 sizes to mul.

[jtmott-intel]

clang/lib/Sema/SemaChecking.cpp

1740–1741

Correct. I passed the BuiltinID so that in SemaBuiltinOverflow I could check for mul specifically. Alternatively, I could give mul its own case "block" separate from add/sub. Thoughts/preferences?

[jtmott-intel]

Limited diagnostic to *signed* _ExtInt, and added test to verify unsigned works.
Reused existing diagnostic message rather than make a new one.

[jtmott-intel]

clang/include/clang/Basic/DiagnosticSemaKinds.td

7951

I discovered there's a good existing message I could use that already takes the bitwidth as a parameter. I decided not to add a new one. Thoughts/preferences? Here's how the message would come out.

test.c:5:43: error: signed _ExtInt of bit sizes greater than 128 not supported
    _Bool status = __builtin_mul_overflow(x, y, &result);
                                          ^
1 error generated.

[rjmccall]

clang/include/clang/Basic/DiagnosticSemaKinds.td

7951

It'd be much clearer to say something about the fact that it's just this builtin that's not supported.

[erichkeane]

clang/lib/Sema/SemaChecking.cpp

325

Disallow _signed_ ...

329

Since you're always doing "Arg.get" below, this should probably just store the Expr*.

330

Comments must end in a period.

332

Try:

Type *Ty = ArgExpr->getType()->getPointeeOrArrayElementType();

instead of the ternary.

335

remove the curley brackets, and make this:

return S.Diag(....

clang/test/Sema/builtins-overflow.c

39

As @rjmccall said, I'd suggest the new message anyway that mentions the builtin, otherwise this would be confusing for me. Something like:

signed argument to __builtin_mul_overflow must have a bitwidth of less than or equal to 128.

[jtmott-intel]

clang/lib/Sema/SemaChecking.cpp

332

A complication I'm seeing is that getPointeeOrArrayElementType returns a Type but getIntWidth wants a QualType. Some options that occurred to me:

• Is it acceptable to just wrap a Type in a QualType?
• I might be able to add a getIntWidth(const Type*) overload.
• Looks like I can skip the getAs<PointerType>() and just call getPointeeType directly. The line would be shorter but I would still need the ternary.

[rjmccall]

clang/lib/Sema/SemaChecking.cpp

322–323

I know this is existing code, but this is a broken mess. Please change this to:

ExprResult Arg = DefaultFunctionArrayLvalueConversion(TheCall->getArg(2));
if (Arg.isInvalid()) return true;
TheCall->setArg(2, Arg.get());

QualType Ty = Arg.get()->getType();
const auto *PtrTy = Ty->getAs<PointerType>();
if (!PtrTy ||
    !PtrTy->getPointeeType()->isIntegerType() ||
    PtrTy->getPointeeType().isConstQualified()) {
  S.Diag(Arg.get()->getBeginLoc(),
         diag::err_overflow_builtin_must_be_ptr_int)
    << Ty << Arg.get()->getSourceRange();
  return true;
}

Test case would be something like (in ObjC):

@interface HasPointer
@property int *pointer;
@end

void test(HasPointer *hp) {
  __builtin_add_overflow(x, y, hp.pointer);
}

And the earlier block needs essentially the same change (but obviously checking for a different type). You can't check types before doing placeholder conversions, and once you do l-value conversions and a type-check, you really don't need the full parameter-initialization logic.

332

• Is it acceptable to just wrap a Type in a QualType?

In general, no, this can lose qualifiers on the array element. But getIntWidth shouldn't ever care about that, so in the abstract, adding the overload should be fine. However, in this case I think the ternary is actually better code.

[jtmott-intel]

This updated patch prepared before John's latest comments.

• Changed diagnostic message to something halfway between John and Erich's suggestions.
• Removed superfluous calls to Arg.get.
• Combined call to Diag and return of error status.
• Simplified but kept ternary (for now).
• Updated comments.

[jtmott-intel]

clang/include/clang/Basic/DiagnosticSemaKinds.td

7951

Updated message to something halfway between John and Erich's suggestions. Current result:

test.c:5:43: error: __builtin_mul_overflow does not support signed _ExtInt operands of more than 128 bits
    _Bool status = __builtin_mul_overflow(x, y, &result);
                                          ^
1 error generated.

clang/test/Sema/builtins-overflow.c

39

Updated message to something halfway between your and John's suggestions. Current result:

test.c:5:43: error: __builtin_mul_overflow does not support signed _ExtInt operands of more than 128 bits
    _Bool status = __builtin_mul_overflow(x, y, &result);
                                          ^
1 error generated.

[rjmccall]

clang/test/Sema/builtins-overflow.c

39

That looks great to me, thanks.

[erichkeane]

Feel free to simplify the error message on commit.

clang/include/clang/Basic/DiagnosticSemaKinds.td

7952

I don't think it is worth doing a 'select' on signed/unsigned. When we need it, we can add it.

I more expect the builtin name to be modified first though (but don't bother making it a modifyable either).

[jtmott-intel]

clang/lib/Sema/SemaChecking.cpp

322–323

I've implemented this locally but I have a quick question about the test. It passed even before I applied this code change. Is this test expected to fail before this change and pass after? Or is it just to prove that the feature (placeholder argument types?) works in general?

[rjmccall]

clang/lib/Sema/SemaChecking.cpp

322–323

Oh, we seem to handle placeholders specially, nevermind. Well, what I said about doing conversions before checking the type still holds, but the test case is just this:

int z[1];
__builtin_add_overflow(x, y, z);

where we should decay the argument to a pointer instead of rejecting it.

[jtmott-intel]

• Removed sign/unsign select.
• Added test and support for placeholder types in builtins.

[jtmott-intel]

clang/lib/Sema/SemaChecking.cpp

322–323

Done. Latest updated patch includes these code changes.

[rjmccall]

LGTM, thanks!

[jtmott-intel]

I don't have commit access. For whoever performs the commit, here's a (draft) commit message:

[clang] Fix or emit diagnostic for checked arithmetic builtins with _ExtInt types

- Fix computed size for _ExtInt types passed to checked arithmetic builtins.
- Emit diagnostic when signed _ExtInt larger than 128-bits is passed to __builtin_mul_overflow.
- Change Sema checks for builtins to accept placeholder types.

clang/include/clang/Basic/DiagnosticSemaKinds.td

def warn_atomic_implicit_seq_cst : Warning<
   "implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary">,
   InGroup<DiagGroup<"atomic-implicit-seq-cst">>, DefaultIgnore;
 
 def err_overflow_builtin_must_be_int : Error<
   "operand argument to overflow builtin must be an integer (%0 invalid)">;
 def err_overflow_builtin_must_be_ptr_int : Error<
   "result argument to overflow builtin must be a pointer "
   "to a non-const integer (%0 invalid)">;
+def err_overflow_builtin_ext_int_max_size : Error<
+  "__builtin_mul_overflow does not support signed _ExtInt operands of more "
+  "than %0 bits">;
 
 def err_atomic_load_store_uses_lib : Error<
   "atomic %select{load|store}0 requires runtime support that is not "
   "available for this target">;
 
 def err_nontemporal_builtin_must_be_pointer : Error<
   "address argument to nontemporal builtin must be a pointer (%0 invalid)">;
 def err_nontemporal_builtin_must_be_pointer_intfltptr_or_vector : Error<

clang/lib/CodeGen/CGBuiltin.cpp

struct WidthAndSignedness {
     bool Signed;
   };
 }
 
 static WidthAndSignedness
 getIntegerWidthAndSignedness(const clang::ASTContext &context,
                              const clang::QualType Type) {
   assert(Type->isIntegerType() && "Given type is not an integer.");
-  unsigned Width = Type->isBooleanType() ? 1 : context.getTypeInfo(Type).Width;
+  unsigned Width = Type->isBooleanType()  ? 1
+                   : Type->isExtIntType() ? context.getIntWidth(Type)
+                                          : context.getTypeInfo(Type).Width;
   bool Signed = Type->isSignedIntegerType();
   return {Width, Signed};
 }
 
 // Given one or more integer types, this function produces an integer type that
 // encompasses them: any value in one of the given types could be expressed in
 // the encompassing type.
 static struct WidthAndSignedness

clang/lib/Sema/SemaChecking.cpp

static bool SemaBuiltinAlignment(Sema &S, CallExpr *TheCall, unsigned ID) {
   TheCall->setArg(1, AlignArg.get());
   // For align_up/align_down, the return type is the same as the (potentially
   // decayed) argument type including qualifiers. For is_aligned(), the result
   // is always bool.
   TheCall->setType(IsBooleanAlignBuiltin ? S.Context.BoolTy : SrcTy);
   return false;
 }
 
-static bool SemaBuiltinOverflow(Sema &S, CallExpr *TheCall) {
+static bool SemaBuiltinOverflow(Sema &S, CallExpr *TheCall,
+                                unsigned BuiltinID) {
   if (checkArgCount(S, TheCall, 3))
     return true;
 
   // First two arguments should be integers.
   for (unsigned I = 0; I < 2; ++I) {
-    ExprResult Arg = TheCall->getArg(I);
+    ExprResult Arg = S.DefaultFunctionArrayLvalueConversion(TheCall->getArg(I));
+    if (Arg.isInvalid()) return true;
+    TheCall->setArg(I, Arg.get());
+
     QualType Ty = Arg.get()->getType();
     if (!Ty->isIntegerType()) {
       S.Diag(Arg.get()->getBeginLoc(), diag::err_overflow_builtin_must_be_int)
           << Ty << Arg.get()->getSourceRange();
       return true;
     }
-    InitializedEntity Entity = InitializedEntity::InitializeParameter(
-        S.getASTContext(), Ty, /*consume*/ false);
-    Arg = S.PerformCopyInitialization(Entity, SourceLocation(), Arg);
-    if (Arg.isInvalid())
-      return true;
-    TheCall->setArg(I, Arg.get());
   }
 
   // Third argument should be a pointer to a non-const integer.
   // IRGen correctly handles volatile, restrict, and address spaces, and
   // the other qualifiers aren't possible.
   {
-    ExprResult Arg = TheCall->getArg(2);
+    ExprResult Arg = S.DefaultFunctionArrayLvalueConversion(TheCall->getArg(2));
+    if (Arg.isInvalid()) return true;
+    TheCall->setArg(2, Arg.get());
+
     QualType Ty = Arg.get()->getType();
     const auto *PtrTy = Ty->getAs<PointerType>();
-    if (!(PtrTy && PtrTy->getPointeeType()->isIntegerType() &&
-          !PtrTy->getPointeeType().isConstQualified())) {
+    if (!PtrTy ||
+        !PtrTy->getPointeeType()->isIntegerType() ||
+        PtrTy->getPointeeType().isConstQualified()) {
       S.Diag(Arg.get()->getBeginLoc(),
              diag::err_overflow_builtin_must_be_ptr_int)
-          << Ty << Arg.get()->getSourceRange();
       return true;
     }
-    InitializedEntity Entity = InitializedEntity::InitializeParameter(
-        S.getASTContext(), Ty, /*consume*/ false);
-    Arg = S.PerformCopyInitialization(Entity, SourceLocation(), Arg);
-    if (Arg.isInvalid())
-      return true;
-    TheCall->setArg(2, Arg.get());
   }
+
+  // Disallow signed ExtIntType args larger than 128 bits to mul function until
+  // we improve backend support.
+  if (BuiltinID == Builtin::BI__builtin_mul_overflow) {
+    for (unsigned I = 0; I < 3; ++I) {
+      const auto Arg = TheCall->getArg(I);
+      // Third argument will be a pointer.
+      auto Ty = I < 2 ? Arg->getType() : Arg->getType()->getPointeeType();
+      if (Ty->isExtIntType() && Ty->isSignedIntegerType() &&
+          S.getASTContext().getIntWidth(Ty) > 128)
+        return S.Diag(Arg->getBeginLoc(),
+                      diag::err_overflow_builtin_ext_int_max_size)
+               << 128;
+    }
+  }
+
   return false;
 }
 
 static bool SemaBuiltinCallWithStaticChain(Sema &S, CallExpr *BuiltinCall) {
   if (checkArgCount(S, BuiltinCall, 2))
     return true;
 
   SourceLocation BuiltinLoc = BuiltinCall->getBeginLoc();
if (SemaBuiltinAddressof(*this, TheCall))
       return ExprError();
     break;
   case Builtin::BI__builtin_is_aligned:
   case Builtin::BI__builtin_align_up:
   case Builtin::BI__builtin_align_down:
     if (SemaBuiltinAlignment(*this, TheCall, BuiltinID))
       return ExprError();
     break;
   case Builtin::BI__builtin_add_overflow:
   case Builtin::BI__builtin_sub_overflow:
   case Builtin::BI__builtin_mul_overflow:
-    if (SemaBuiltinOverflow(*this, TheCall))
+    if (SemaBuiltinOverflow(*this, TheCall, BuiltinID))
       return ExprError();
     break;
   case Builtin::BI__builtin_operator_new:
   case Builtin::BI__builtin_operator_delete: {
     bool IsDelete = BuiltinID == Builtin::BI__builtin_operator_delete;
     ExprResult Res =
         SemaBuiltinOperatorNewDeleteOverloaded(TheCallResult, IsDelete);
     if (Res.isInvalid())

clang/test/CodeGen/builtins-overflow.c

int test_add_overflow_int_int_int(int x, int y) {
   // CHECK: store i32 [[Q]], i32*
   // CHECK: br i1 [[C]]
   int r;
   if (__builtin_add_overflow(x, y, &r))
     overflowed();
   return r;
 }
 
+int test_add_overflow_xint31_xint31_xint31(_ExtInt(31) x, _ExtInt(31) y) {
+  // CHECK-LABEL: define {{(dso_local )?}}i32 @test_add_overflow_xint31_xint31_xint31({{.+}})
+  // CHECK-NOT: ext
+  // CHECK: [[S:%.+]] = call { i31, i1 } @llvm.sadd.with.overflow.i31(i31 %{{.+}}, i31 %{{.+}})
+  // CHECK-DAG: [[C:%.+]] = extractvalue { i31, i1 } [[S]], 1
+  // CHECK-DAG: [[Q:%.+]] = extractvalue { i31, i1 } [[S]], 0
+  // CHECK: store i31 [[Q]], i31*
+  // CHECK: br i1 [[C]]
+  _ExtInt(31) r;
+  if (__builtin_add_overflow(x, y, &r))
+    overflowed();
+  return r;
+}
+
 unsigned test_sub_overflow_uint_uint_uint(unsigned x, unsigned y) {
   // CHECK-LABEL: define {{(dso_local )?}}i32 @test_sub_overflow_uint_uint_uint
   // CHECK-NOT: ext
   // CHECK: [[S:%.+]] = call { i32, i1 } @llvm.usub.with.overflow.i32(i32 %{{.+}}, i32 %{{.+}})
   // CHECK-DAG: [[Q:%.+]] = extractvalue { i32, i1 } [[S]], 0
   // CHECK-DAG: [[C:%.+]] = extractvalue { i32, i1 } [[S]], 1
   // CHECK: store i32 [[Q]], i32*
   // CHECK: br i1 [[C]]
int test_sub_overflow_int_int_int(int x, int y) {
   // CHECK: store i32 [[Q]], i32*
   // CHECK: br i1 [[C]]
   int r;
   if (__builtin_sub_overflow(x, y, &r))
     overflowed();
   return r;
 }
 
+int test_sub_overflow_xint31_xint31_xint31(_ExtInt(31) x, _ExtInt(31) y) {
+  // CHECK-LABEL: define {{(dso_local )?}}i32 @test_sub_overflow_xint31_xint31_xint31({{.+}})
+  // CHECK-NOT: ext
+  // CHECK: [[S:%.+]] = call { i31, i1 } @llvm.ssub.with.overflow.i31(i31 %{{.+}}, i31 %{{.+}})
+  // CHECK-DAG: [[C:%.+]] = extractvalue { i31, i1 } [[S]], 1
+  // CHECK-DAG: [[Q:%.+]] = extractvalue { i31, i1 } [[S]], 0
+  // CHECK: store i31 [[Q]], i31*
+  // CHECK: br i1 [[C]]
+  _ExtInt(31) r;
+  if (__builtin_sub_overflow(x, y, &r))
+    overflowed();
+  return r;
+}
+
 unsigned test_mul_overflow_uint_uint_uint(unsigned x, unsigned y) {
   // CHECK-LABEL: define {{(dso_local )?}}i32 @test_mul_overflow_uint_uint_uint
   // CHECK-NOT: ext
   // CHECK: [[S:%.+]] = call { i32, i1 } @llvm.umul.with.overflow.i32(i32 %{{.+}}, i32 %{{.+}})
   // CHECK-DAG: [[Q:%.+]] = extractvalue { i32, i1 } [[S]], 0
   // CHECK-DAG: [[C:%.+]] = extractvalue { i32, i1 } [[S]], 1
   // CHECK: store i32 [[Q]], i32*
   // CHECK: br i1 [[C]]
int test_mul_overflow_int_int_int(int x, int y) {
   // CHECK: store i32 [[Q]], i32*
   // CHECK: br i1 [[C]]
   int r;
   if (__builtin_mul_overflow(x, y, &r))
     overflowed();
   return r;
 }
 
+int test_mul_overflow_xint31_xint31_xint31(_ExtInt(31) x, _ExtInt(31) y) {
+  // CHECK-LABEL: define {{(dso_local )?}}i32 @test_mul_overflow_xint31_xint31_xint31({{.+}})
+  // CHECK-NOT: ext
+  // CHECK: [[S:%.+]] = call { i31, i1 } @llvm.smul.with.overflow.i31(i31 %{{.+}}, i31 %{{.+}})
+  // CHECK-DAG: [[C:%.+]] = extractvalue { i31, i1 } [[S]], 1
+  // CHECK-DAG: [[Q:%.+]] = extractvalue { i31, i1 } [[S]], 0
+  // CHECK: store i31 [[Q]], i31*
+  // CHECK: br i1 [[C]]
+  _ExtInt(31) r;
+  if (__builtin_mul_overflow(x, y, &r))
+    overflowed();
+  return r;
+}
+
+int test_mul_overflow_xint127_xint127_xint127(_ExtInt(127) x, _ExtInt(127) y) {
+  // CHECK-LABEL: define {{(dso_local )?}}i32 @test_mul_overflow_xint127_xint127_xint127({{.+}})
+  // CHECK-NOT: ext
+  // CHECK: [[S:%.+]] = call { i127, i1 } @llvm.smul.with.overflow.i127(i127 %{{.+}}, i127 %{{.+}})
+  // CHECK-DAG: [[C:%.+]] = extractvalue { i127, i1 } [[S]], 1
+  // CHECK-DAG: [[Q:%.+]] = extractvalue { i127, i1 } [[S]], 0
+  // CHECK: store i127 [[Q]], i127*
+  // CHECK: br i1 [[C]]
+  _ExtInt(127) r;
+  if (__builtin_mul_overflow(x, y, &r))
+    overflowed();
+  return r;
+}
+
+int test_mul_overflow_xint128_xint128_xint128(_ExtInt(128) x, _ExtInt(128) y) {
+  // CHECK-LABEL: define {{(dso_local )?}}i32 @test_mul_overflow_xint128_xint128_xint128({{.+}})
+  // CHECK-NOT: ext
+  // CHECK: [[S:%.+]] = call { i128, i1 } @llvm.smul.with.overflow.i128(i128 %{{.+}}, i128 %{{.+}})
+  // CHECK-DAG: [[C:%.+]] = extractvalue { i128, i1 } [[S]], 1
+  // CHECK-DAG: [[Q:%.+]] = extractvalue { i128, i1 } [[S]], 0
+  // CHECK: store i128 [[Q]], i128*
+  // CHECK: br i1 [[C]]
+  _ExtInt(128) r;
+  if (__builtin_mul_overflow(x, y, &r))
+    overflowed();
+  return r;
+}
+
 int test_add_overflow_uint_int_int(unsigned x, int y) {
   // CHECK-LABEL: define {{(dso_local )?}}i32 @test_add_overflow_uint_int_int
   // CHECK: [[XE:%.+]] = zext i32 %{{.+}} to i33
   // CHECK: [[YE:%.+]] = sext i32 %{{.+}} to i33
   // CHECK: [[S:%.+]] = call { i33, i1 } @llvm.sadd.with.overflow.i33(i33 [[XE]], i33 [[YE]])
   // CHECK-DAG: [[Q:%.+]] = extractvalue { i33, i1 } [[S]], 0
   // CHECK-DAG: [[C1:%.+]] = extractvalue { i33, i1 } [[S]], 1
   // CHECK: [[QT:%.+]] = trunc i33 [[Q]] to i32

clang/test/Sema/builtins-overflow.c

void test(void) {
   __builtin_add_overflow();  // expected-error {{too few arguments to function call, expected 3, have 0}}
   __builtin_add_overflow(1, 1, 1, 1);  // expected-error {{too many arguments to function call, expected 3, have 4}}
 
   __builtin_add_overflow(c, 1, &r);  // expected-error {{operand argument to overflow builtin must be an integer ('const char *' invalid)}}
   __builtin_add_overflow(1, c, &r);  // expected-error {{operand argument to overflow builtin must be an integer ('const char *' invalid)}}
   __builtin_add_overflow(1, 1, 3);  // expected-error {{result argument to overflow builtin must be a pointer to a non-const integer ('int' invalid)}}
   __builtin_add_overflow(1, 1, &f);  // expected-error {{result argument to overflow builtin must be a pointer to a non-const integer ('float *' invalid)}}
   __builtin_add_overflow(1, 1, &q);  // expected-error {{result argument to overflow builtin must be a pointer to a non-const integer ('const unsigned int *' invalid)}}
+
+  {
+    _ExtInt(128) x = 1;
+    _ExtInt(128) y = 1;
+    _ExtInt(128) result;
+    _Bool status = __builtin_mul_overflow(x, y, &result); // expect ok
+  }
+  {
+    unsigned _ExtInt(129) x = 1;
+    unsigned _ExtInt(129) y = 1;
+    unsigned _ExtInt(129) result;
+    _Bool status = __builtin_mul_overflow(x, y, &result); // expect ok
+  }
+  {
+    _ExtInt(129) x = 1;
+    _ExtInt(129) y = 1;
+    _ExtInt(129) result;
+    _Bool status = __builtin_mul_overflow(x, y, &result); // expected-error {{__builtin_mul_overflow does not support signed _ExtInt operands of more than 128 bits}}
+  }
 }

clang/test/Sema/builtins-overflow.m

+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+void test() {
+  int z[1];
+  __builtin_add_overflow(1, 1, z);
+}