jtmott-intel created this revision.
jtmott-intel added reviewers: erichkeane, rjmccall.
This patch addresses two issues.
The first issue is use of the checked arithmetic builtins
<https://clang.llvm.org/docs/LanguageExtensions.html#checked-arithmetic-builtins>
in combination with _ExtInt
<http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2472.pdf> 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.
https://reviews.llvm.org/D81420
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Sema/SemaChecking.cpp
clang/test/CodeGen/builtins-overflow.c
Index: clang/test/CodeGen/builtins-overflow.c
===================================================================
--- clang/test/CodeGen/builtins-overflow.c
+++ clang/test/CodeGen/builtins-overflow.c
@@ -41,6 +41,20 @@
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
@@ -69,6 +83,20 @@
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
@@ -97,6 +125,20 @@
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_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
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -283,7 +283,8 @@
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;
@@ -325,6 +326,24 @@
return true;
TheCall->setArg(2, Arg.get());
}
+
+ // Disallow ExtIntType args larger than 64 bits to mul function until we
+ // improve backend support.
+ if (BuiltinID == Builtin::BI__builtin_mul_overflow) {
+ for (unsigned I = 0; I < 3; ++I) {
+ ExprResult Arg = TheCall->getArg(I);
+ // Third argument will be a pointer
+ QualType Ty =
+ I < 2 ? Arg.get()->getType()
+ : Arg.get()->getType()->getAs<PointerType>()->getPointeeType();
+ if (Ty->isExtIntType() && S.getASTContext().getIntWidth(Ty) > 64) {
+ S.Diag(Arg.get()->getBeginLoc(), diag::err_overflow_builtin_extint_size)
+ << Ty << Arg.get()->getSourceRange();
+ return true;
+ }
+ }
+ }
+
return false;
}
@@ -1728,7 +1747,7 @@
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:
Index: clang/lib/CodeGen/CGBuiltin.cpp
===================================================================
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -586,7 +586,9 @@
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};
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7936,6 +7936,9 @@
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_extint_size : Error<
+ "_ExtInt argument larger than 64-bits to overflow builtin requires runtime "
+ "support that is not available for this target">;
def err_atomic_load_store_uses_lib : Error<
"atomic %select{load|store}0 requires runtime support that is not "
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
