Bug 229332 - Add some offlineasm enhancements.
Summary: Add some offlineasm enhancements.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on: 229449
Blocks:
  Show dependency treegraph
 
Reported: 2021-08-20 03:18 PDT by Mark Lam
Modified: 2021-08-24 19:35 PDT (History)
7 users (show)

See Also:


Attachments
proposed patch. (8.72 KB, patch)
2021-08-20 03:27 PDT, Mark Lam
keith_miller: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2021-08-20 03:18:56 PDT
And also fix a minor LLInt code alignment bug.
Comment 1 Radar WebKit Bug Importer 2021-08-20 03:19:16 PDT
<rdar://problem/82163923>
Comment 2 Mark Lam 2021-08-20 03:27:06 PDT
Created attachment 435966 [details]
proposed patch.
Comment 3 Keith Miller 2021-08-20 09:08:54 PDT
Comment on attachment 435966 [details]
proposed patch.

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

r=me with nits

> Source/JavaScriptCore/ChangeLog:10
> +           file from /usr/local/include/WebKitAdditions/ first.  If the specified file is

Nit: This is technically <build-products>/usr/local/include/WebKitAdditions.

> Source/JavaScriptCore/offlineasm/parser.rb:266
> +        @buildProductsDirectory = ENV['BUILT_PRODUCTS_DIR'];

Can you add a FIXME for CMake support here with a bug? I'm fairly sure CMake doesn't set that environment variable.
Comment 4 Mark Lam 2021-08-20 09:59:19 PDT
Thanks for the review.

(In reply to Keith Miller from comment #3)
> > Source/JavaScriptCore/ChangeLog:10
> > +           file from /usr/local/include/WebKitAdditions/ first.  If the specified file is
> 
> Nit: This is technically <build-products>/usr/local/include/WebKitAdditions.

Fixed.

> > Source/JavaScriptCore/offlineasm/parser.rb:266
> > +        @buildProductsDirectory = ENV['BUILT_PRODUCTS_DIR'];
> 
> Can you add a FIXME for CMake support here with a bug? I'm fairly sure CMake
> doesn't set that environment variable.

Done.  Ref: https://bugs.webkit.org/show_bug.cgi?id=229340
Comment 5 Mark Lam 2021-08-20 10:01:35 PDT
Landed in r281321: <http://trac.webkit.org/r281321>.
Comment 6 WebKit Commit Bot 2021-08-24 09:54:33 PDT
Re-opened since this is blocked by bug 229449
Comment 7 Mark Lam 2021-08-24 19:35:49 PDT
Re-landed in r281541: <http://trac.webkit.org/r281541>.