Bug 198453 – Reenable Gigacage on ARM64.

• Status: REOPENED
• Component: JavaScriptCore

[Keith Miller]

Reenable Gigacage on ARM64.

[Keith Miller]

Created attachment 371113 [details]
Patch

[Keith Miller]

Comment on attachment 371113 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:11
> +        Gigacaging would otherwise strip a PAC failed authenticate bit we
> +        force a load of the pointer into some garbage register.

I was thinking this would be free because we would load the length from the base of the pointer anyway. Unfortunately, that is only true for butterflies and not for TypedArray storage. I'm not sure what the right fix is... perhaps this is still fine but it's probably better to not pollute the caching hierarchy...

[WebKit Commit Bot]

Comment on attachment 371113 [details]
Patch

Clearing flags on attachment: 371113

Committed r246022: <https://trac.webkit.org/changeset/246022>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/51340879>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 198486

[Dean Johnson]

Note: Internal tests also show a 3-5x magnitude performance regression on arm64e, compared to arm64. Probably a bug there.

[Saam Barati]

Gonna take this over since Keith is busy at the moment with standards meetings.

[Keith Miller]

Created attachment 371487 [details]
Patch

[Keith Miller]

(In reply to Saam Barati from comment #8)
> Gonna take this over since Keith is busy at the moment with standards
> meetings.

But I have a patch!

[Keith Miller]

Created attachment 371488 [details]
Patch

[Michael Saboff]

Comment on attachment 371488 [details]
Patch

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

r=me with one comment.

> Source/JavaScriptCore/offlineasm/instructions.rb:276
> +     "bfiq", # Bit field insert <source reg> <width immediate> <last bit written> <dest reg>

The order of width and last bit written seem to be backwards.

[Keith Miller]

Created attachment 371490 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 371490 [details]
Patch for landing

Clearing flags on attachment: 371490

Committed r246150: <https://trac.webkit.org/changeset/246150>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Saam Barati]

Comment on attachment 371490 [details]
Patch for landing

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

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:6846
> +    m_jit.cageWithoutUntaging(Gigacage::JSValue, dataGPR);

“Untaging” => “Untagging”

[Saam Barati]

(In reply to Keith Miller from comment #10)
> (In reply to Saam Barati from comment #8)
> > Gonna take this over since Keith is busy at the moment with standards
> > meetings.
> 
> But I have a patch!

👍🏼

[Saam Barati]

What was the perf regression from the prior patch?

[Keith Miller]

(In reply to Saam Barati from comment #18)
> What was the perf regression from the prior patch?

I was forcing a load of the untagged PC before passing it off to the gigacage stripping code.

[Keith Miller]

Comment on attachment 371490 [details]
Patch for landing

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

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:6846
>> +    m_jit.cageWithoutUntaging(Gigacage::JSValue, dataGPR);
> 
> “Untaging” => “Untagging”

I uploaded a patch to fix the typo:  https://bugs.webkit.org/show_bug.cgi?id=198617

[WebKit Commit Bot]

Re-opened since this is blocked by bug 198698