Bug 230803

Summary: [JSC][32bit] Fix build after unlinked baseline JIT (r283083)
Product: WebKit Reporter: Xan Lopez <xan.lopez>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, ews-watchlist, gyuyoung.kim, keith_miller, kevin_neal, mark.lam, msaboff, ryuan.choi, saam, sergio, tzagallo, webkit-bug-importer, xan.lopez
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP
none
v1
ews-feeder: commit-queue-
v2
ews-feeder: commit-queue-
v3
ews-feeder: commit-queue-
v4
saam: review+
v5
xan.lopez: commit-queue+
v6 none

Description Xan Lopez 2021-09-26 01:54:44 PDT
Bug #229223 broke 32-bit builds.
Comment 1 Xan Lopez 2021-09-30 07:16:08 PDT
Created attachment 439734 [details]
WIP

This compiles/links/very basic stuff works, but it's still wrong and incomplete.
Comment 2 Kevin Neal 2021-09-30 15:11:49 PDT
Thank you for filing. The appropriate engineers have been notified.
Comment 3 Radar WebKit Bug Importer 2021-09-30 15:12:01 PDT
<rdar://problem/83739988>
Comment 4 Xan Lopez 2021-10-01 06:44:12 PDT
Created attachment 439849 [details]
v1
Comment 5 Xan Lopez 2021-10-01 06:50:39 PDT
Created attachment 439851 [details]
v2

Try to fix i386 build.
Comment 6 Xan Lopez 2021-10-01 07:05:01 PDT
Created attachment 439855 [details]
v3

Add missing header to CMakeLists.txt
Comment 7 Xan Lopez 2021-10-01 07:23:32 PDT
Created attachment 439857 [details]
v4

And another missing header... (RegisterMap.h)
Comment 8 Saam Barati 2021-10-01 10:22:32 PDT
Comment on attachment 439857 [details]
v4

View in context: https://bugs.webkit.org/attachment.cgi?id=439857&action=review

Seems like a good first step to get things building. r=me

> Source/JavaScriptCore/bytecode/CallLinkInfo.h:28
> +#include "CallFrameShuffleData.h"

why was this needed. Why didn't the forward declare work?

> Source/JavaScriptCore/jit/JITInlines.h:369
> +    UNUSED_PARAM(value);
> +    UNUSED_PARAM(bytecode);
> +    // FIXME.

I think this can just be:
emitValueProfilingSite(valueProfileFor(bytecode.metadata(m_profiledCodeBlock), m_bytecodeIndex.checkpoint()), value);

> Source/JavaScriptCore/jit/JITOpcodes.cpp:1472
> +        loadGlobalObject(resultRegs.payloadGPR()); // FIXME: need to move the whole thing.

I think you just need:
move(TrustedImm32(CellTag), resultRegs.tagGPR())
Comment 9 Xan Lopez 2021-10-01 10:33:55 PDT
(In reply to Saam Barati from comment #8)
> Comment on attachment 439857 [details]
> v4
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=439857&action=review
> 
> Seems like a good first step to get things building. r=me
> 
> > Source/JavaScriptCore/bytecode/CallLinkInfo.h:28
> > +#include "CallFrameShuffleData.h"
> 
> why was this needed. Why didn't the forward declare work?

std::unique_ptr was complaining, but I read in the spec that it explicitly allows for incomplete types. I'll try again.
Comment 10 Xan Lopez 2021-10-01 10:49:59 PDT
Created attachment 439876 [details]
v5

patch to land
Comment 11 Xan Lopez 2021-10-01 10:54:21 PDT
Created attachment 439877 [details]
v6

v6, patch to land (credit a couple of colleagues who helped with this patch)
Comment 12 EWS 2021-10-01 12:06:13 PDT
Committed r283389 (242393@main): <https://commits.webkit.org/242393@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 439877 [details].