[webkit-reviews] review granted: [Bug 228047] Add Pre/Post-Indexed Address Mode to Air for ARM64 : [Attachment 434582] Patch

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has granted Yijia Huang
<yijia_huang at apple.com>'s request for review:
Bug 228047: Add Pre/Post-Indexed Address Mode to Air for ARM64
https://bugs.webkit.org/show_bug.cgi?id=228047

Attachment 434582: Patch

https://bugs.webkit.org/attachment.cgi?id=434582&action=review




--- Comment #31 from Filip Pizlo <fpizlo at apple.com> ---
Comment on attachment 434582
  --> https://bugs.webkit.org/attachment.cgi?id=434582
Patch

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

I think this is right?

> Source/JavaScriptCore/b3/B3CanonicalizePrePostIncrements.cpp:167
> +    return canonicalizePrePostIncrementsImpl(proc);

You could inline the body of the impl here.

> Source/JavaScriptCore/b3/B3CanonicalizePrePostIncrements.h:34
> +bool canonicalizePrePostIncrementsImpl(Procedure&);

Don't need this declared here since you don't call it from anywhere but the
non-impl variant.

> Source/JavaScriptCore/b3/air/AirArg.h:1398
> +	   case PreIndex:
> +	   case PostIndex:
> +	       functor(m_base, UseDef, GP, argRole == UseAddr ? argWidth :
pointerWidth());
> +	       break;

Fingers crossed that this works!

Ideally we'd audit the compiler to make sure nobody is assuming that only a
top-level Arg can be a Def.