This is an archive of the discontinued LLVM Phabricator instance.

Additionally, these all have the 't' flag, which means that these signatures are meaningless, right? What are you seeing where this works incorrectly?

In D52875#1255146, @erichkeane wrote:

Can you write tests for this please? Particularly validate the results in a constexpr context.

There are already some tests for those builtins (not sure about constexpr context). They already tests that the builtins can be used as branching condition. However, the current implementation of Sema::BuildResolvedCallExpr assumes that by default builtins return bool. In https://reviews.llvm.org/D52879, I improve that and not having the above fix makes the existing tests fail, so I believe we don't need to add more tests.

Additionally, these all have the 't' flag, which means that these signatures are meaningless, right? What are you seeing where this works incorrectly?

I reckon the signature does't include the return type, hence it isn't meaningless even with the t flag.

erichkeane accepted this revision.Oct 4 2018, 6:46 AM

This revision is now accepted and ready to land.Oct 4 2018, 6:46 AM

Closed by commit rC344891: [OpenCL] Fix definitions of __builtin_(add|sub|mul)_overflow (authored by mantognini). · Explain WhyOct 22 2018, 3:43 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents

Path

Size

include/

clang/

Basic/

Builtins.def

6 lines

Diff 170383

include/clang/Basic/Builtins.def

	Show First 20 Lines • Show All 1,392 Lines • ▼ Show 20 Lines
	BUILTIN(__builtin_addcll, "ULLiULLiCULLiCULLiCULLi*", "n")			BUILTIN(__builtin_addcll, "ULLiULLiCULLiCULLiCULLi*", "n")
	BUILTIN(__builtin_subcb, "UcUcCUcCUcCUc*", "n")			BUILTIN(__builtin_subcb, "UcUcCUcCUcCUc*", "n")
	BUILTIN(__builtin_subcs, "UsUsCUsCUsCUs*", "n")			BUILTIN(__builtin_subcs, "UsUsCUsCUsCUs*", "n")
	BUILTIN(__builtin_subc, "UiUiCUiCUiCUi*", "n")			BUILTIN(__builtin_subc, "UiUiCUiCUiCUi*", "n")
	BUILTIN(__builtin_subcl, "ULiULiCULiCULiCULi*", "n")			BUILTIN(__builtin_subcl, "ULiULiCULiCULiCULi*", "n")
	BUILTIN(__builtin_subcll, "ULLiULLiCULLiCULLiCULLi*", "n")			BUILTIN(__builtin_subcll, "ULLiULLiCULLiCULLiCULLi*", "n")

	// Checked Arithmetic Builtins for Security.			// Checked Arithmetic Builtins for Security.
	BUILTIN(__builtin_add_overflow, "v.", "nt")			BUILTIN(__builtin_add_overflow, "b.", "nt")
	BUILTIN(__builtin_sub_overflow, "v.", "nt")			BUILTIN(__builtin_sub_overflow, "b.", "nt")
	BUILTIN(__builtin_mul_overflow, "v.", "nt")			BUILTIN(__builtin_mul_overflow, "b.", "nt")
	BUILTIN(__builtin_uadd_overflow, "bUiCUiCUi*", "n")			BUILTIN(__builtin_uadd_overflow, "bUiCUiCUi*", "n")
	BUILTIN(__builtin_uaddl_overflow, "bULiCULiCULi*", "n")			BUILTIN(__builtin_uaddl_overflow, "bULiCULiCULi*", "n")
	BUILTIN(__builtin_uaddll_overflow, "bULLiCULLiCULLi*", "n")			BUILTIN(__builtin_uaddll_overflow, "bULLiCULLiCULLi*", "n")
	BUILTIN(__builtin_usub_overflow, "bUiCUiCUi*", "n")			BUILTIN(__builtin_usub_overflow, "bUiCUiCUi*", "n")
	BUILTIN(__builtin_usubl_overflow, "bULiCULiCULi*", "n")			BUILTIN(__builtin_usubl_overflow, "bULiCULiCULi*", "n")
	BUILTIN(__builtin_usubll_overflow, "bULLiCULLiCULLi*", "n")			BUILTIN(__builtin_usubll_overflow, "bULLiCULLiCULLi*", "n")
	BUILTIN(__builtin_umul_overflow, "bUiCUiCUi*", "n")			BUILTIN(__builtin_umul_overflow, "bUiCUiCUi*", "n")
	BUILTIN(__builtin_umull_overflow, "bULiCULiCULi*", "n")			BUILTIN(__builtin_umull_overflow, "bULiCULiCULi*", "n")
	▲ Show 20 Lines • Show All 109 Lines • Show Last 20 Lines

This is an archive of the discontinued LLVM Phabricator instance.

Fix definitions of __builtin_(add|sub|mul)_overflowClosedPublic

Details

Diff Detail

Event Timeline

Revision Contents

Diff 170383

include/clang/Basic/Builtins.def

Fix definitions of __builtin_(add|sub|mul)_overflow
ClosedPublic