Bug 181525 – JITMathIC code in the FTL is wrong when code gets duplicated

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Saam Barati]

B3 may duplicate code for arbitrary reasons. The code in FTLLower should generate a new mathIC per invocation of the watchpoint callback.

[Saam Barati]

<rdar://problem/36351993>

[Saam Barati]

Created attachment 331043 [details]
patch

[Saam Barati]

Created attachment 331079 [details]
patch

[Michael Saboff]

Comment on attachment 331079 [details]
patch

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

r=me

> Source/JavaScriptCore/bytecode/CodeBlock.h:267
>      JITAddIC* addJITAddIC(ArithProfile*);
>      JITMulIC* addJITMulIC(ArithProfile*);
> -    JITNegIC* addJITNegIC(ArithProfile*);
>      JITSubIC* addJITSubIC(ArithProfile*);
> +    JITNegIC* addJITNegIC(ArithProfile*);
> +
> +    template <typename Generator, typename = typename std::enable_if<std::is_same<Generator, JITAddGenerator>::value>::type>
> +    JITAddIC* addMathIC(ArithProfile* profile) { return addJITAddIC(profile); }
> +
> +    template <typename Generator, typename = typename std::enable_if<std::is_same<Generator, JITSubGenerator>::value>::type>
> +    JITSubIC* addMathIC(ArithProfile* profile) { return addJITSubIC(profile); }
> +
> +    template <typename Generator, typename = typename std::enable_if<std::is_same<Generator, JITMulGenerator>::value>::type>
> +    JITMulIC* addMathIC(ArithProfile* profile) { return addJITMulIC(profile); }
> +
> +    template <typename Generator, typename = typename std::enable_if<std::is_same<Generator, JITNegGenerator>::value>::type>
> +    JITNegIC* addMathIC(ArithProfile* profile) { return addJITNegIC(profile); }

The declarations were alphabetical.  Now the declarations and template declarations have different orders and don't appear to follow any order.

[Keith Miller]

Comment on attachment 331079 [details]
patch

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

r=me.

> Source/JavaScriptCore/bytecode/CodeBlock.h:257
> +    template <typename Generator, typename = typename std::enable_if<std::is_same<Generator, JITAddGenerator>::value>::type>

Ugg... we should really make a macro for this...

[Saam Barati]

Comment on attachment 331079 [details]
patch

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

>> Source/JavaScriptCore/bytecode/CodeBlock.h:257
>> +    template <typename Generator, typename = typename std::enable_if<std::is_same<Generator, JITAddGenerator>::value>::type>
> 
> Ugg... we should really make a macro for this...

I still want my constexpr if! I can't wait

>> Source/JavaScriptCore/bytecode/CodeBlock.h:267
>> +    JITNegIC* addMathIC(ArithProfile* profile) { return addJITNegIC(profile); }
> 
> The declarations were alphabetical.  Now the declarations and template declarations have different orders and don't appear to follow any order.

Will fix.

[Saam Barati]

Created attachment 331087 [details]
patch for landing

[Saam Barati]

Created attachment 331088 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 331088 [details]
patch for landing

Clearing flags on attachment: 331088

Committed r226806: <https://trac.webkit.org/changeset/226806>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.