Skip to content

Conversation

@Constellation Constellation requested a review from a team as a code owner January 22, 2024 22:12
@Constellation Constellation self-assigned this Jan 22, 2024
@Constellation Constellation added the JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. label Jan 22, 2024
@Constellation Constellation added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jan 22, 2024
https://bugs.webkit.org/show_bug.cgi?id=267881
rdar://121391447

* Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:
(JSC::CLoop::execute):

Canonical link: https://commits.webkit.org/273318@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Unreviewed-build-fix-for-CLoop branch from d96d86c to c1f8a9e Compare January 22, 2024 22:17
@webkit-commit-queue webkit-commit-queue merged commit c1f8a9e into WebKit:main Jan 22, 2024
@webkit-commit-queue
Copy link
Collaborator

Committed 273318@main (c1f8a9e): https://commits.webkit.org/273318@main

Reviewed commits have been landed. Closing PR #23066 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jan 22, 2024
@Constellation Constellation deleted the eng/Unreviewed-build-fix-for-CLoop branch January 22, 2024 22:18
@balducci
Copy link

balducci commented Feb 6, 2024

hi

Committed 273318@main (c1f8a9e): https://commits.webkit.org/273318@main

Reviewed commits have been landed. Closing PR #23066 and removing active labels.

cross building for 32 bit on a 64 bit host (under linux) now fails due to what appears to be a typo (t6 t7 used but not declared):

    /home/balducci/tmp/install-us-d/webkitgtk-2.42.5/webkitgtk-2.42.5/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:339:21: error: ‘t6’ was not declared in this scope; did you mean ‘tm’?
      339 |     UNUSED_VARIABLE(t6);
          |                     ^~
    [...]
    /home/balducci/tmp/install-us-d/webkitgtk-2.42.5/webkitgtk-2.42.5/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:340:21: error: ‘t7’ was not declared in this scope; did you mean ‘tm’?
      340 |     UNUSED_VARIABLE(t7);
          |                     ^~

This fixes things for me:

diff -c Source/JavaScriptCore/llint/LowLevelInterpreter.cpp.FIX_T6_T7 Source/JavaScriptCore/llint/LowLevelInterpreter.cpp
*** Source/JavaScriptCore/llint/LowLevelInterpreter.cpp.FIX_T6_T7	2024-02-06 14:42:22.103792982 +0100
--- Source/JavaScriptCore/llint/LowLevelInterpreter.cpp	2024-02-06 14:42:22.107793035 +0100
***************
*** 323,329 ****
      // 2. 32 bit result values will be in the low 32-bit of t0.
      // 3. 64 bit result values will be in t0.
  
!     CLoopRegister t0, t1, t2, t3, t5, sp, cfr, lr, pc;
  #if USE(JSVALUE64)
      CLoopRegister numberTag, notCellMask;
  #endif
--- 323,329 ----
      // 2. 32 bit result values will be in the low 32-bit of t0.
      // 3. 64 bit result values will be in t0.
  
!     CLoopRegister t0, t1, t2, t3, t5, t6, t7, sp, cfr, lr, pc;
  #if USE(JSVALUE64)
      CLoopRegister numberTag, notCellMask;
  #endif

ciao
gabriele

@sthen
Copy link

sthen commented Feb 9, 2024

cross building for 32 bit on a 64 bit host (under linux) now fails due to what appears to be a typo (t6 t7 used but not declared):

    /home/balducci/tmp/install-us-d/webkitgtk-2.42.5/webkitgtk-2.42.5/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:339:21: error: ‘t6’ was not declared in this scope; did you mean ‘tm’?
      339 |     UNUSED_VARIABLE(t6);
          |                     ^~
    [...]
    /home/balducci/tmp/install-us-d/webkitgtk-2.42.5/webkitgtk-2.42.5/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:340:21: error: ‘t7’ was not declared in this scope; did you mean ‘tm’?
      340 |     UNUSED_VARIABLE(t7);
          |                     ^~

Seeing this in native builds on OpenBSD i386 (32-bit) too.

This fixes things for me:

diff -c Source/JavaScriptCore/llint/LowLevelInterpreter.cpp.FIX_T6_T7 Source/JavaScriptCore/llint/LowLevelInterpreter.cpp
*** Source/JavaScriptCore/llint/LowLevelInterpreter.cpp.FIX_T6_T7	2024-02-06 14:42:22.103792982 +0100
--- Source/JavaScriptCore/llint/LowLevelInterpreter.cpp	2024-02-06 14:42:22.107793035 +0100
***************
*** 323,329 ****
      // 2. 32 bit result values will be in the low 32-bit of t0.
      // 3. 64 bit result values will be in t0.
  
!     CLoopRegister t0, t1, t2, t3, t5, sp, cfr, lr, pc;
  #if USE(JSVALUE64)
      CLoopRegister numberTag, notCellMask;
  #endif
--- 323,329 ----
      // 2. 32 bit result values will be in the low 32-bit of t0.
      // 3. 64 bit result values will be in t0.
  
!     CLoopRegister t0, t1, t2, t3, t5, t6, t7, sp, cfr, lr, pc;
  #if USE(JSVALUE64)
      CLoopRegister numberTag, notCellMask;
  #endif

Shouldn't the "UNUSED_VARIABLE(t6);" and "UNUSED_VARIABLE(t7);" be removed instead?

@bitlord
Copy link

bitlord commented Feb 19, 2024

@balducci @sthen webkit bug: https://bugs.webkit.org/show_bug.cgi?id=268739

Based on the reference in the commit of webkitglib/2.42 branch I think this should fix it 3d53735

@sthen
Copy link

sthen commented Feb 19, 2024 via email

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

Labels

JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants