[clang] constexpr built-in abs function. #112539

c8ef · 2024-10-16T13:14:13Z

According to P0533R9, the C++ standard library functions corresponding to the C macros in [c.math.abs] are now constexpr.

To implement this feature in libc++, we must make the built-in abs function constexpr. This patch adds the implementation of a constexpr abs function for the current constant evaluator and the new bytecode interpreter.

It is important to note that in 2's complement systems, the absolute value of the most negative value is out of range. In gcc, it will result in an out-of-range error and will not be evaluated as constants. We follow the same approach here.

llvmbot · 2024-10-16T14:36:29Z

@llvm/pr-subscribers-clang

Author: None (c8ef)

Changes

According to P0533R9, the C++ standard library functions corresponding to the C macros in [c.math.abs] are now constexpr.

To implement this feature in libc++, we must make the built-in abs function constexpr. This patch adds the implementation of a constexpr abs function for the current constant evaluator and the new bytecode interpreter.

It is important to note that in 2's complement systems, the absolute value of the most negative value is out of range. In gcc, it will result in an out-of-range error and will not be evaluated as constants. We follow the same approach here.

Full diff: https://github.com/llvm/llvm-project/pull/112539.diff

6 Files Affected:

(modified) clang/include/clang/Basic/Builtins.td (+1)
(modified) clang/lib/AST/ByteCode/InterpBuiltin.cpp (+20)
(modified) clang/lib/AST/ExprConstant.cpp (+13)
(modified) clang/test/AST/ByteCode/builtin-functions.cpp (+9)
(modified) clang/test/CodeGenCXX/builtins.cpp (+6-8)
(modified) clang/test/Sema/constant-builtins-2.c (+12-1)

diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td
index bda8a48be92bda..e1b4d5b1fdc0a5 100644
--- a/clang/include/clang/Basic/Builtins.td
+++ b/clang/include/clang/Basic/Builtins.td
@@ -2714,6 +2714,7 @@ def Abs : IntMathTemplate, LibBuiltin<"stdlib.h"> {
   let Attributes = [NoThrow, Const];
   let Prototype = "T(T)";
   let AddBuiltinPrefixedAlias = 1;
+  let OnlyBuiltinPrefixedAliasIsConstexpr = 1;
 }
 
 def Calloc : LibBuiltin<"stdlib.h"> {
diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
index 65c7b4e5306d72..b4ab744ea14104 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
@@ -563,6 +563,19 @@ static bool interp__builtin_fabs(InterpState &S, CodePtr OpPC,
   return true;
 }
 
+static bool interp__builtin_abs(InterpState &S, CodePtr OpPC,
+                                const InterpFrame *Frame, const Function *Func,
+                                const CallExpr *Call) {
+  PrimType ArgT = *S.getContext().classify(Call->getArg(0)->getType());
+  APSInt Val = peekToAPSInt(S.Stk, ArgT);
+  if (Val == APSInt(APSInt::getSignedMinValue(Val.getBitWidth()), false))
+    return false;
+  if (Val.isNegative())
+    Val.negate();
+  pushInteger(S, Val, Call->getType());
+  return true;
+}
+
 static bool interp__builtin_popcount(InterpState &S, CodePtr OpPC,
                                      const InterpFrame *Frame,
                                      const Function *Func,
@@ -1808,6 +1821,13 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
       return false;
     break;
 
+  case Builtin::BI__builtin_abs:
+  case Builtin::BI__builtin_labs:
+  case Builtin::BI__builtin_llabs:
+    if (!interp__builtin_abs(S, OpPC, Frame, F, Call))
+      return false;
+    break;
+
   case Builtin::BI__builtin_popcount:
   case Builtin::BI__builtin_popcountl:
   case Builtin::BI__builtin_popcountll:
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 52a7f5778ce6d2..d1d6790e152527 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -13055,6 +13055,19 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
     return Success(Val.popcount() % 2, E);
   }
 
+  case Builtin::BI__builtin_abs:
+  case Builtin::BI__builtin_labs:
+  case Builtin::BI__builtin_llabs: {
+    APSInt Val;
+    if (!EvaluateInteger(E->getArg(0), Val, Info))
+      return false;
+    if (Val == APSInt(APSInt::getSignedMinValue(Val.getBitWidth()), false))
+      return false;
+    if (Val.isNegative())
+      Val.negate();
+    return Success(Val, E);
+  }
+
   case Builtin::BI__builtin_popcount:
   case Builtin::BI__builtin_popcountl:
   case Builtin::BI__builtin_popcountll:
diff --git a/clang/test/AST/ByteCode/builtin-functions.cpp b/clang/test/AST/ByteCode/builtin-functions.cpp
index 450ff5671314db..1ec9b5fdcbd228 100644
--- a/clang/test/AST/ByteCode/builtin-functions.cpp
+++ b/clang/test/AST/ByteCode/builtin-functions.cpp
@@ -265,6 +265,15 @@ namespace fpclassify {
   char classify_subnorm [__builtin_fpclassify(-1, -1, -1, +1, -1, 1.0e-38f)];
 }
 
+namespace abs {
+static_assert(__builtin_abs(14) == 14, "");
+static_assert(__builtin_labs(14L) == 14L, "");
+static_assert(__builtin_llabs(14LL) == 14LL, "");
+static_assert(__builtin_abs(-14) == 14, "");
+static_assert(__builtin_labs(-0x14L) == 0x14L, "");
+static_assert(__builtin_llabs(-0x141414141414LL) == 0x141414141414LL, "");
+} // namespace abs
+
 namespace fabs {
   static_assert(__builtin_fabs(-14.0) == 14.0, "");
 }
diff --git a/clang/test/CodeGenCXX/builtins.cpp b/clang/test/CodeGenCXX/builtins.cpp
index 90265186fb3d8c..37f9491d12d04b 100644
--- a/clang/test/CodeGenCXX/builtins.cpp
+++ b/clang/test/CodeGenCXX/builtins.cpp
@@ -14,6 +14,12 @@ int o = X::__builtin_fabs(-2.0);
 long p = X::__builtin_fabsf(-3.0f);
 // CHECK: @p ={{.*}} global i64 3, align 8
 
+int x = __builtin_abs(-2);
+// CHECK: @x ={{.*}} global i32 2, align 4
+
+long y = __builtin_abs(-2l);
+// CHECK: @y ={{.*}} global i64 2, align 8
+
 // PR8839
 extern "C" char memmove();
 
@@ -52,14 +58,6 @@ extern "C" int __builtin_abs(int); // #1
 long __builtin_abs(long);          // #2
 extern "C" int __builtin_abs(int); // #3
 
-int x = __builtin_abs(-2);
-// CHECK:      [[X:%.+]] = call i32 @llvm.abs.i32(i32 -2, i1 true)
-// CHECK-NEXT: store i32 [[X]], ptr @x, align 4
-
-long y = __builtin_abs(-2l);
-// CHECK:  [[Y:%.+]] = call noundef i64 @_Z13__builtin_absl(i64 noundef -2)
-// CHECK:  store i64 [[Y]], ptr @y, align 8
-
 extern const char char_memchr_arg[32];
 char *memchr_result = __builtin_char_memchr(char_memchr_arg, 123, 32);
 // CHECK: call ptr @memchr(ptr noundef @char_memchr_arg, i32 noundef 123, i64 noundef 32)
diff --git a/clang/test/Sema/constant-builtins-2.c b/clang/test/Sema/constant-builtins-2.c
index da2264500d7680..e465a3c5f0ad86 100644
--- a/clang/test/Sema/constant-builtins-2.c
+++ b/clang/test/Sema/constant-builtins-2.c
@@ -35,7 +35,7 @@ long double  g11 = __builtin_nansl("");
 __float128   g11_2 = __builtin_nansf128("");
 #endif
 
-//int          g12 = __builtin_abs(-12);
+int g12 = __builtin_abs(-12);
 
 double       g13 = __builtin_fabs(-12.);
 double       g13_0 = __builtin_fabs(-0.);
@@ -456,6 +456,17 @@ char clrsb9[__builtin_clrsb(1 << (BITSIZE(int) - 1)) == 0 ? 1 : -1];
 char clrsb10[__builtin_clrsb(~(1 << (BITSIZE(int) - 1))) == 0 ? 1 : -1];
 char clrsb11[__builtin_clrsb(0xf) == BITSIZE(int) - 5 ? 1 : -1];
 char clrsb12[__builtin_clrsb(~0x1f) == BITSIZE(int) - 6 ? 1 : -1];
+
+char abs1[__builtin_abs(-12)];
+char abs2[__builtin_labs(-12L)];
+char abs3[__builtin_llabs(-12LL)];
+int abs4 = __builtin_abs(1 << (BITSIZE(int) - 1)); // expected-error {{not a compile-time constant}}
+char abs5[__builtin_abs((1 << (BITSIZE(int) - 1)) + 1)];
+long abs6 = __builtin_labs(1L << (BITSIZE(long) - 1)); // expected-error {{not a compile-time constant}}
+long abs7 = __builtin_labs((1L << (BITSIZE(long) - 1)) + 1);
+long long abs8 = __builtin_llabs(1LL << (BITSIZE(long long) - 1)); // expected-error {{not a compile-time constant}}
+long long abs9 = __builtin_llabs((1LL << (BITSIZE(long long) - 1)) + 1);
+
 #undef BITSIZE
 
 // GCC misc stuff

clang/test/AST/ByteCode/builtin-functions.cpp

clang/lib/AST/ByteCode/InterpBuiltin.cpp

cor3ntin · 2024-10-17T10:24:49Z

Please add an entry to clang/docs/ReleaseNotes.rst, thanks!

c8ef · 2024-10-17T13:15:40Z

Please add an entry to clang/docs/ReleaseNotes.rst, thanks!

Done.

cor3ntin · 2024-10-17T14:19:38Z

clang/test/AST/ByteCode/builtin-functions.cpp

+namespace abs {
+  static_assert(__builtin_abs(14) == 14, "");
+  static_assert(__builtin_labs(14L) == 14L, "");
+  static_assert(__builtin_llabs(14LL) == 14LL, "");
+  static_assert(__builtin_abs(-14) == 14, "");
+  static_assert(__builtin_labs(-0x14L) == 0x14L, "");
+  static_assert(__builtin_llabs(-0x141414141414LL) == 0x141414141414LL, "");
+} // namespace abs
+


can you add the same error test cases as in the other files (abs4, etc)

cor3ntin

lgtm

c8ef · 2024-10-18T09:22:11Z

Thank you for all your reviews! If there are no further comments, I will proceed with merging this after updating the branch.

c8ef · 2024-10-18T11:03:14Z

Clang :: Analysis/diagnostics/sarif-diagnostics-taint-test.c
Clang :: Analysis/diagnostics/sarif-multi-diagnostic-test.c
Clang :: CXX/drs/cwg3xx.cpp

These test failures are unrelated and have been observed in other CI runs. Let's go ahead and merge this for now.

c8ef added 8 commits October 16, 2024 01:19

implement constexpr builtin {l}abs

dfa1585

Update InterpBuiltin.cpp

52e1d07

Update ExprConstant.cpp

2271935

Update InterpBuiltin.cpp

5f45e1f

Update ExprConstant.cpp

05b6cd4

Update ExprConstant.cpp

12e545f

Update ExprConstant.cpp

0adef10

abs

7881253

c8ef changed the title ~~Draft~~ [clang] constexpr built-in abs function. Oct 16, 2024

add more test

7efd267

c8ef marked this pull request as ready for review October 16, 2024 14:35

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 16, 2024

c8ef requested review from AaronBallman, Endilll and tbaederr October 16, 2024 14:36

c8ef requested a review from cor3ntin October 17, 2024 02:11

tbaederr reviewed Oct 17, 2024

View reviewed changes

clang/test/AST/ByteCode/builtin-functions.cpp Outdated Show resolved Hide resolved

clang/lib/AST/ByteCode/InterpBuiltin.cpp Outdated Show resolved Hide resolved

tbaederr requested a review from Sirraide October 17, 2024 07:15

address review comments

5a4dcbd

c8ef requested a review from tbaederr October 17, 2024 13:18

cor3ntin reviewed Oct 17, 2024

View reviewed changes

add test

89a29c5

c8ef requested a review from cor3ntin October 17, 2024 15:37

cor3ntin approved these changes Oct 17, 2024

View reviewed changes

Merge branch 'main' into abs-cont

79e7e3d

Merge branch 'main' into abs-cont

7fcf937

c8ef merged commit 332ac18 into llvm:main Oct 18, 2024
6 of 7 checks passed

c8ef deleted the abs-cont branch October 18, 2024 14:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[clang] constexpr built-in abs function. #112539

[clang] constexpr built-in abs function. #112539

Uh oh!

c8ef commented Oct 16, 2024 •

edited

Loading

Uh oh!

llvmbot commented Oct 16, 2024

Uh oh!

Uh oh!

Uh oh!

cor3ntin commented Oct 17, 2024

Uh oh!

c8ef commented Oct 17, 2024

Uh oh!

cor3ntin Oct 17, 2024

Uh oh!

c8ef Oct 17, 2024

Uh oh!

cor3ntin left a comment

Uh oh!

c8ef commented Oct 18, 2024

Uh oh!

c8ef commented Oct 18, 2024

Uh oh!

Uh oh!

Uh oh!

[clang] constexpr built-in abs function. #112539

[clang] constexpr built-in abs function. #112539

Uh oh!

Conversation

c8ef commented Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Oct 16, 2024

Uh oh!

Uh oh!

Uh oh!

cor3ntin commented Oct 17, 2024

Uh oh!

c8ef commented Oct 17, 2024

Uh oh!

cor3ntin Oct 17, 2024

Choose a reason for hiding this comment

Uh oh!

c8ef Oct 17, 2024

Choose a reason for hiding this comment

Uh oh!

cor3ntin left a comment

Choose a reason for hiding this comment

Uh oh!

c8ef commented Oct 18, 2024

Uh oh!

c8ef commented Oct 18, 2024

Uh oh!

Uh oh!

Uh oh!

c8ef commented Oct 16, 2024 •

edited

Loading