Skip to content

Conversation

donneypr
Copy link
Contributor

@donneypr donneypr commented Oct 7, 2025

Fixes #160288

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:bytecode Issues for the clang bytecode constexpr interpreter labels Oct 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 7, 2025

@llvm/pr-subscribers-clang

Author: don (donneypr)

Changes

Fixes #160288


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

1 Files Affected:

  • (modified) clang/lib/AST/ByteCode/InterpBuiltin.cpp (+12-32)
diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
index a0dcdace854b9..81e6bd55990fa 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
@@ -671,33 +671,6 @@ static bool interp__builtin_popcount(InterpState &S, CodePtr OpPC,
   return true;
 }
 
-static bool interp__builtin_parity(InterpState &S, CodePtr OpPC,
-                                   const InterpFrame *Frame,
-                                   const CallExpr *Call) {
-  PrimType ArgT = *S.getContext().classify(Call->getArg(0)->getType());
-  APSInt Val = popToAPSInt(S.Stk, ArgT);
-  pushInteger(S, Val.popcount() % 2, Call->getType());
-  return true;
-}
-
-static bool interp__builtin_clrsb(InterpState &S, CodePtr OpPC,
-                                  const InterpFrame *Frame,
-                                  const CallExpr *Call) {
-  PrimType ArgT = *S.getContext().classify(Call->getArg(0)->getType());
-  APSInt Val = popToAPSInt(S.Stk, ArgT);
-  pushInteger(S, Val.getBitWidth() - Val.getSignificantBits(), Call->getType());
-  return true;
-}
-
-static bool interp__builtin_bitreverse(InterpState &S, CodePtr OpPC,
-                                       const InterpFrame *Frame,
-                                       const CallExpr *Call) {
-  PrimType ArgT = *S.getContext().classify(Call->getArg(0)->getType());
-  APSInt Val = popToAPSInt(S.Stk, ArgT);
-  pushInteger(S, Val.reverseBits(), Call->getType());
-  return true;
-}
-
 static bool interp__builtin_classify_type(InterpState &S, CodePtr OpPC,
                                           const InterpFrame *Frame,
                                           const CallExpr *Call) {
@@ -3032,18 +3005,25 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const CallExpr *Call,
   case Builtin::BI__builtin_parity:
   case Builtin::BI__builtin_parityl:
   case Builtin::BI__builtin_parityll:
-    return interp__builtin_parity(S, OpPC, Frame, Call);
-
+    return interp__builtin_elementwise_int_unaryop(
+        S, OpPC, Call, [](const APSInt &Val) -> APInt {
+          return APInt(Val.getBitWidth(), Val.popcount() % 2);
+        });
   case Builtin::BI__builtin_clrsb:
   case Builtin::BI__builtin_clrsbl:
   case Builtin::BI__builtin_clrsbll:
-    return interp__builtin_clrsb(S, OpPC, Frame, Call);
-
+    return interp__builtin_elementwise_int_unaryop(
+        S, OpPC, Call, [](const APSInt &Val) -> APInt {
+          return APInt(Val.getBitWidth(),
+                       Val.getBitWidth() - Val.getSignificantBits());
+        });
   case Builtin::BI__builtin_bitreverse8:
   case Builtin::BI__builtin_bitreverse16:
   case Builtin::BI__builtin_bitreverse32:
   case Builtin::BI__builtin_bitreverse64:
-    return interp__builtin_bitreverse(S, OpPC, Frame, Call);
+    return interp__builtin_elementwise_int_unaryop(
+        S, OpPC, Call,
+        [](const APSInt &Val) -> APInt { return Val.reverseBits(); });
 
   case Builtin::BI__builtin_classify_type:
     return interp__builtin_classify_type(S, OpPC, Frame, Call);

@donneypr donneypr marked this pull request as draft October 7, 2025 19:20
@donneypr donneypr marked this pull request as ready for review October 7, 2025 19:41
Copy link

github-actions bot commented Oct 7, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@donneypr
Copy link
Contributor Author

donneypr commented Oct 7, 2025

/home/gha/actions-runner/_work/llvm-project/llvm-project/clang/lib/AST/ByteCode/InterpBuiltin.cpp:716:13: error: unused function 'interp__builtin_rotate' [-Werror,-Wunused-function]

Looks like this function is not used thus the Linux Build and Test failing, please let me know if this is accurate.

@donneypr
Copy link
Contributor Author

donneypr commented Oct 7, 2025

/home/gha/actions-runner/_work/llvm-project/llvm-project/clang/lib/AST/ByteCode/InterpBuiltin.cpp:716:13: error: unused function 'interp__builtin_rotate' [-Werror,-Wunused-function]

Looks like this function is not used thus the Linux Build and Test failing, please let me know if this is accurate.

Nevermind, I think that was accidentally added when I was merging. The callback for interp__builtin_rotate was replaced in #160289.

@donneypr
Copy link
Contributor Author

donneypr commented Oct 7, 2025

@tbaederr @RKSimon Can I please get a review? Thank you 😊 !!

@RKSimon RKSimon requested review from RKSimon and tbaederr October 7, 2025 21:18
Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - cheers

@RKSimon RKSimon enabled auto-merge (squash) October 8, 2025 14:12
@donneypr donneypr requested a review from tbaederr October 8, 2025 14:13
@RKSimon RKSimon merged commit d0da857 into llvm:main Oct 8, 2025
9 checks passed
@donneypr
Copy link
Contributor Author

donneypr commented Oct 8, 2025

Thank you @RKSimon @tbaederr , have a great rest of your day!!

svkeerthy pushed a commit that referenced this pull request Oct 9, 2025
…e/ffs with static bool interp__builtin_elementwise_int_unaryop callback (#162346)

Fixes #160288
clingfei pushed a commit to clingfei/llvm-project that referenced this pull request Oct 10, 2025
…e/ffs with static bool interp__builtin_elementwise_int_unaryop callback (llvm#162346)

Fixes llvm#160288
case Builtin::BI__builtin_ffsll:
return interp__builtin_ffs(S, OpPC, Frame, Call);
return interp__builtin_elementwise_int_unaryop(
S, OpPC, Call, [](const APSInt &Val) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious, you only left off the trailing return type on this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:bytecode Issues for the clang bytecode constexpr interpreter clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[clang][x86][bytecode] Replace interp__builtin_parity/clrsb/bitreverse/ffs with static bool interp__builtin_elementwise_int_unaryop callback

5 participants