Skip to content

Conversation

RKSimon
Copy link
Collaborator

@RKSimon RKSimon commented Oct 8, 2025

The interp__builtin_ia32_pmadd implementation can be correctly used for PMULDQ/PMULUDQ evaluation as well as we're ignoring the "hi" integers in each pair

I've replaced the PMULDQ/PMULUDQ evaluation with callbacks and renamed interp__builtin_ia32_pmadd to interp__builtin_ia32_pmul for consistency

…n_ia32_pmadd implementations

The interp__builtin_ia32_pmadd implementation can be correctly used for PMULDQ/PMULUDQ evaluation as well as we're ignoring the "hi" integers in each pair

I've replaced the PMULDQ/PMULUDQ evaluation with callbacks and renamed interp__builtin_ia32_pmadd to interp__builtin_ia32_pmul for consistency
@RKSimon RKSimon requested a review from tbaederr October 8, 2025 16:12
@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 8, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 8, 2025

@llvm/pr-subscribers-clang

Author: Simon Pilgrim (RKSimon)

Changes

The interp__builtin_ia32_pmadd implementation can be correctly used for PMULDQ/PMULUDQ evaluation as well as we're ignoring the "hi" integers in each pair

I've replaced the PMULDQ/PMULUDQ evaluation with callbacks and renamed interp__builtin_ia32_pmadd to interp__builtin_ia32_pmul for consistency


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

1 Files Affected:

  • (modified) clang/lib/AST/ByteCode/InterpBuiltin.cpp (+16-52)
diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
index 9125250b77347..922d67940e22f 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
@@ -2549,7 +2549,7 @@ static bool interp__builtin_elementwise_maxmin(InterpState &S, CodePtr OpPC,
   return true;
 }
 
-static bool interp__builtin_ia32_pmadd(
+static bool interp__builtin_ia32_pmul(
     InterpState &S, CodePtr OpPC, const CallExpr *Call,
     llvm::function_ref<APInt(const APSInt &, const APSInt &, const APSInt &,
                              const APSInt &)>
@@ -2587,54 +2587,6 @@ static bool interp__builtin_ia32_pmadd(
   return true;
 }
 
-static bool interp__builtin_ia32_pmul(InterpState &S, CodePtr OpPC,
-                                      const CallExpr *Call,
-                                      unsigned BuiltinID) {
-  assert(Call->getArg(0)->getType()->isVectorType() &&
-         Call->getArg(1)->getType()->isVectorType());
-  const Pointer &RHS = S.Stk.pop<Pointer>();
-  const Pointer &LHS = S.Stk.pop<Pointer>();
-  const Pointer &Dst = S.Stk.peek<Pointer>();
-
-  const auto *VT = Call->getArg(0)->getType()->castAs<VectorType>();
-  PrimType ElemT = *S.getContext().classify(VT->getElementType());
-  unsigned SourceLen = VT->getNumElements();
-
-  PrimType DstElemT = *S.getContext().classify(
-      Call->getType()->castAs<VectorType>()->getElementType());
-  unsigned DstElem = 0;
-  for (unsigned I = 0; I != SourceLen; I += 2) {
-    APSInt Elem1;
-    APSInt Elem2;
-    INT_TYPE_SWITCH_NO_BOOL(ElemT, {
-      Elem1 = LHS.elem<T>(I).toAPSInt();
-      Elem2 = RHS.elem<T>(I).toAPSInt();
-    });
-
-    APSInt Result;
-    switch (BuiltinID) {
-    case clang::X86::BI__builtin_ia32_pmuludq128:
-    case clang::X86::BI__builtin_ia32_pmuludq256:
-    case clang::X86::BI__builtin_ia32_pmuludq512:
-      Result = APSInt(llvm::APIntOps::muluExtended(Elem1, Elem2),
-                      /*IsUnsigned=*/true);
-      break;
-    case clang::X86::BI__builtin_ia32_pmuldq128:
-    case clang::X86::BI__builtin_ia32_pmuldq256:
-    case clang::X86::BI__builtin_ia32_pmuldq512:
-      Result = APSInt(llvm::APIntOps::mulsExtended(Elem1, Elem2),
-                      /*IsUnsigned=*/false);
-      break;
-    }
-    INT_TYPE_SWITCH_NO_BOOL(DstElemT,
-                            { Dst.elem<T>(DstElem) = static_cast<T>(Result); });
-    ++DstElem;
-  }
-
-  Dst.initializeAllElements();
-  return true;
-}
-
 static bool interp__builtin_elementwise_triop_fp(
     InterpState &S, CodePtr OpPC, const CallExpr *Call,
     llvm::function_ref<APFloat(const APFloat &, const APFloat &,
@@ -3512,7 +3464,7 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const CallExpr *Call,
   case clang::X86::BI__builtin_ia32_pmaddubsw128:
   case clang::X86::BI__builtin_ia32_pmaddubsw256:
   case clang::X86::BI__builtin_ia32_pmaddubsw512:
-    return interp__builtin_ia32_pmadd(
+    return interp__builtin_ia32_pmul(
         S, OpPC, Call,
         [](const APSInt &LoLHS, const APSInt &HiLHS, const APSInt &LoRHS,
            const APSInt &HiRHS) {
@@ -3524,7 +3476,7 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const CallExpr *Call,
   case clang::X86::BI__builtin_ia32_pmaddwd128:
   case clang::X86::BI__builtin_ia32_pmaddwd256:
   case clang::X86::BI__builtin_ia32_pmaddwd512:
-    return interp__builtin_ia32_pmadd(
+    return interp__builtin_ia32_pmul(
         S, OpPC, Call,
         [](const APSInt &LoLHS, const APSInt &HiLHS, const APSInt &LoRHS,
            const APSInt &HiRHS) {
@@ -3677,10 +3629,22 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const CallExpr *Call,
   case clang::X86::BI__builtin_ia32_pmuldq128:
   case clang::X86::BI__builtin_ia32_pmuldq256:
   case clang::X86::BI__builtin_ia32_pmuldq512:
+    return interp__builtin_ia32_pmul(
+        S, OpPC, Call,
+        [](const APSInt &LoLHS, const APSInt &HiLHS, const APSInt &LoRHS,
+           const APSInt &HiRHS) {
+          return llvm::APIntOps::mulsExtended(LoLHS, LoRHS);
+        });
+
   case clang::X86::BI__builtin_ia32_pmuludq128:
   case clang::X86::BI__builtin_ia32_pmuludq256:
   case clang::X86::BI__builtin_ia32_pmuludq512:
-    return interp__builtin_ia32_pmul(S, OpPC, Call, BuiltinID);
+    return interp__builtin_ia32_pmul(
+        S, OpPC, Call,
+        [](const APSInt &LoLHS, const APSInt &HiLHS, const APSInt &LoRHS,
+           const APSInt &HiRHS) {
+          return llvm::APIntOps::muluExtended(LoLHS, LoRHS);
+        });
 
   case Builtin::BI__builtin_elementwise_fma:
     return interp__builtin_elementwise_triop_fp(

@RKSimon RKSimon merged commit 7f8d9b0 into llvm:main Oct 9, 2025
13 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Oct 9, 2025

LLVM Buildbot has detected a new failure on builder clang-aarch64-sve-vla running on linaro-g3-02 while building clang at step 7 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/17/builds/11736

Here is the relevant piece of the build log for the reference
Step 7 (ninja check 1) failure: stage 1 checked (failure)
******************** TEST 'SanitizerCommon-lsan-aarch64-Linux :: compress_stack_depot.cpp' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
/home/tcwg-buildbot/worker/clang-aarch64-sve-vla/stage1/./bin/clang  --driver-mode=g++ -gline-tables-only -fsanitize=leak   -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta  -funwind-tables  -I/home/tcwg-buildbot/worker/clang-aarch64-sve-vla/llvm/compiler-rt/test -ldl /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/llvm/compiler-rt/test/sanitizer_common/TestCases/compress_stack_depot.cpp -fsanitize-memory-track-origins=1 -o /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/stage1/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-aarch64-Linux/Output/compress_stack_depot.cpp.tmp # RUN: at line 1
+ /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/stage1/./bin/clang --driver-mode=g++ -gline-tables-only -fsanitize=leak -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta -funwind-tables -I/home/tcwg-buildbot/worker/clang-aarch64-sve-vla/llvm/compiler-rt/test -ldl /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/llvm/compiler-rt/test/sanitizer_common/TestCases/compress_stack_depot.cpp -fsanitize-memory-track-origins=1 -o /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/stage1/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-aarch64-Linux/Output/compress_stack_depot.cpp.tmp
clang: warning: argument unused during compilation: '-fsanitize-memory-track-origins=1' [-Wunused-command-line-argument]
env LSAN_OPTIONS="compress_stack_depot=0:malloc_context_size=128:verbosity=1"  /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/stage1/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-aarch64-Linux/Output/compress_stack_depot.cpp.tmp 2>&1 | FileCheck /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/llvm/compiler-rt/test/sanitizer_common/TestCases/compress_stack_depot.cpp --implicit-check-not="StackDepot released" # RUN: at line 2
+ env LSAN_OPTIONS=compress_stack_depot=0:malloc_context_size=128:verbosity=1 /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/stage1/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-aarch64-Linux/Output/compress_stack_depot.cpp.tmp
+ FileCheck /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/llvm/compiler-rt/test/sanitizer_common/TestCases/compress_stack_depot.cpp '--implicit-check-not=StackDepot released'
env LSAN_OPTIONS="compress_stack_depot=-1:malloc_context_size=128:verbosity=1"  /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/stage1/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-aarch64-Linux/Output/compress_stack_depot.cpp.tmp 2>&1 | FileCheck /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/llvm/compiler-rt/test/sanitizer_common/TestCases/compress_stack_depot.cpp --check-prefixes=COMPRESS # RUN: at line 3
+ env LSAN_OPTIONS=compress_stack_depot=-1:malloc_context_size=128:verbosity=1 /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/stage1/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-aarch64-Linux/Output/compress_stack_depot.cpp.tmp
+ FileCheck /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/llvm/compiler-rt/test/sanitizer_common/TestCases/compress_stack_depot.cpp --check-prefixes=COMPRESS
env LSAN_OPTIONS="compress_stack_depot=-2:malloc_context_size=128:verbosity=1"  /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/stage1/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-aarch64-Linux/Output/compress_stack_depot.cpp.tmp 2>&1 | FileCheck /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/llvm/compiler-rt/test/sanitizer_common/TestCases/compress_stack_depot.cpp --check-prefixes=COMPRESS # RUN: at line 4
+ FileCheck /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/llvm/compiler-rt/test/sanitizer_common/TestCases/compress_stack_depot.cpp --check-prefixes=COMPRESS
+ env LSAN_OPTIONS=compress_stack_depot=-2:malloc_context_size=128:verbosity=1 /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/stage1/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-aarch64-Linux/Output/compress_stack_depot.cpp.tmp
env LSAN_OPTIONS="compress_stack_depot=1:malloc_context_size=128:verbosity=1"  /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/stage1/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-aarch64-Linux/Output/compress_stack_depot.cpp.tmp 2>&1 | FileCheck /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/llvm/compiler-rt/test/sanitizer_common/TestCases/compress_stack_depot.cpp --check-prefixes=COMPRESS,THREAD # RUN: at line 5
+ env LSAN_OPTIONS=compress_stack_depot=1:malloc_context_size=128:verbosity=1 /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/stage1/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-aarch64-Linux/Output/compress_stack_depot.cpp.tmp
+ FileCheck /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/llvm/compiler-rt/test/sanitizer_common/TestCases/compress_stack_depot.cpp --check-prefixes=COMPRESS,THREAD
/home/tcwg-buildbot/worker/clang-aarch64-sve-vla/llvm/compiler-rt/test/sanitizer_common/TestCases/compress_stack_depot.cpp:53:14: error: COMPRESS: expected string not found in input
// COMPRESS: StackDepot released {{[0-9]+}}
             ^
<stdin>:12:53: note: scanning from here
LeakSanitizer: StackDepot compression thread started
                                                    ^
<stdin>:13:16: note: possible intended match here
LeakSanitizer: StackDepot compression thread stopped
               ^

Input file: <stdin>
Check file: /home/tcwg-buildbot/worker/clang-aarch64-sve-vla/llvm/compiler-rt/test/sanitizer_common/TestCases/compress_stack_depot.cpp

-dump-input=help explains the following input dump.

Input was:
<<<<<<
            .
            .
            .
            7: ==512910==Unregistered root region at 0xece9fd403190 of size 208 
            8: ==512910==Unregistered root region at 0xece9fde00780 of size 32 
            9: ==512910==Installed the sigaction for signal 11 
           10: ==512910==Installed the sigaction for signal 7 
           11: ==512910==Installed the sigaction for signal 8 
           12: LeakSanitizer: StackDepot compression thread started 
check:53'0                                                         X error: no match found
           13: LeakSanitizer: StackDepot compression thread stopped 
check:53'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
check:53'1                    ?                                      possible intended match
...

@RKSimon RKSimon deleted the x86-bytecode-pmul-pmadd branch October 9, 2025 08:09
svkeerthy pushed a commit that referenced this pull request Oct 9, 2025
…n_ia32_pmadd implementations (#162504)

The interp__builtin_ia32_pmadd implementation can be correctly used for
PMULDQ/PMULUDQ evaluation as well as we're ignoring the "hi" integers in
each pair

I've replaced the PMULDQ/PMULUDQ evaluation with callbacks and renamed
interp__builtin_ia32_pmadd to interp__builtin_ia32_pmul for consistency
clingfei pushed a commit to clingfei/llvm-project that referenced this pull request Oct 10, 2025
…n_ia32_pmadd implementations (llvm#162504)

The interp__builtin_ia32_pmadd implementation can be correctly used for
PMULDQ/PMULUDQ evaluation as well as we're ignoring the "hi" integers in
each pair

I've replaced the PMULDQ/PMULUDQ evaluation with callbacks and renamed
interp__builtin_ia32_pmadd to interp__builtin_ia32_pmul for consistency
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.

4 participants