Bug 225787

Summary: m_calleeSaveRegisters should not be a pointer to a pointer
Product: WebKit Reporter: Geoffrey Garen <ggaren>
Component: New BugsAssignee: Geoffrey Garen <ggaren>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, nham, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Geoffrey Garen 2021-05-13 16:23:53 PDT
m_calleeSaveRegisters should not be a pointer to a pointer
Comment 1 Geoffrey Garen 2021-05-13 16:27:35 PDT
Created attachment 428573 [details]
Patch
Comment 2 Keith Miller 2021-05-13 16:31:00 PDT
Comment on attachment 428573 [details]
Patch

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

r=me with nits.

> Source/JavaScriptCore/bytecode/CodeBlock.h:287
> +        bool m_hasCalleeSaveRegisters { false };

Can't we just add an operator bool to RegisterAtOffsetList which is true when the FixedVector has a pointer?
Comment 3 Geoffrey Garen 2021-05-13 16:41:43 PDT
> > Source/JavaScriptCore/bytecode/CodeBlock.h:287
> > +        bool m_hasCalleeSaveRegisters { false };
> 
> Can't we just add an operator bool to RegisterAtOffsetList which is true
> when the FixedVector has a pointer?

If we did that, we wouldn't be able to distinguish "not initialized" from "initialized with 0 size". I'm not sure if that matters or not.
Comment 4 Geoffrey Garen 2021-05-13 16:42:10 PDT
....so I took the conservative approach and maintained support for "initialized with 0 size".
Comment 5 Geoffrey Garen 2021-05-13 18:27:31 PDT
Comment on attachment 428573 [details]
Patch

CQ+
Comment 6 Keith Miller 2021-05-13 18:55:21 PDT
(In reply to Geoffrey Garen from comment #4)
> ....so I took the conservative approach and maintained support for
> "initialized with 0 size".

Ah yes, all those platforms with 0 calleeSavedRegisters we support 🙃
Comment 7 EWS 2021-05-13 19:03:51 PDT
Committed r277475 (237710@main): <https://commits.webkit.org/237710@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 428573 [details].
Comment 8 Radar WebKit Bug Importer 2021-05-13 19:04:15 PDT
<rdar://problem/77998921>