[JSC] Add Float support to B3
Created attachment 266835 [details] Patch
*** Bug 151921 has been marked as a duplicate of this bug. ***
*** Bug 151903 has been marked as a duplicate of this bug. ***
Attachment 266835 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/b3/testb3.cpp:5937: l is incorrectly named. Don't use the single letter 'l' as an identifier name. [readability/naming] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6340: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6341: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6342: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6343: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6344: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6345: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6377: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6378: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6379: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6380: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6381: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6382: More than one command on the same line [whitespace/newline] [4] Total errors found: 13 in 38 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 266836 [details] Patch
Attachment 266836 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/b3/testb3.cpp:5937: l is incorrectly named. Don't use the single letter 'l' as an identifier name. [readability/naming] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6340: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6341: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6342: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6343: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6344: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6345: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6377: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6378: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6379: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6380: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6381: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6382: More than one command on the same line [whitespace/newline] [4] Total errors found: 13 in 38 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 266837 [details] Patch
Attachment 266837 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/b3/testb3.cpp:5937: l is incorrectly named. Don't use the single letter 'l' as an identifier name. [readability/naming] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6340: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6341: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6342: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6343: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6344: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6345: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6377: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6378: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6379: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6380: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6381: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:6382: More than one command on the same line [whitespace/newline] [4] Total errors found: 13 in 38 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 266837 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=266837&action=review I think you're just missing moveFloatConditionally, which corresponds to Float = Select(Int, Float, Float). But I guess that depends on whether or not you believe that movsd can efficiently be used instead of movss. > Source/JavaScriptCore/ChangeLog:8 > + This patch adds basic float support to B3. Basic or comprehensive? > Source/JavaScriptCore/assembler/MacroAssembler.h:1365 > + > + void moveDoubleConditionallyFloat(DoubleCondition cond, FPRegisterID left, FPRegisterID right, FPRegisterID src, FPRegisterID dest) > + { > + Jump falseCase = branchFloat(invert(cond), left, right); > + moveDouble(src, dest); > + falseCase.link(this); > + } What about moveFloatConditionally? > Source/JavaScriptCore/b3/B3Generate.cpp:78 > + reduceDoubleToFloat(procedure); > + Any particular reason why this isn't run after reduceStrength()? Actually, why isn't this just part of reduceStrength()? > Source/JavaScriptCore/b3/B3LowerToAir.cpp:783 > + case Float: This doesn't seem right. Is it really optimal to use movsd when we just need movss?
Comment on attachment 266837 [details] Patch We talked about the moveFloat/moveFloatConditionally issue offline.
Comment on attachment 266837 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=266837&action=review > Source/JavaScriptCore/b3/B3ReduceDoubleToFloat.cpp:85 > + HashSet<Value*> candidates; Why not IndexSet<Value> candidates?
Committed r193683: <http://trac.webkit.org/changeset/193683>
Comment on attachment 266837 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=266837&action=review > Source/JavaScriptCore/b3/B3ReduceDoubleToFloat.cpp:72 > +// Such operation arise in JS because there are is no Float type are is > Source/JavaScriptCore/b3/B3ReduceDoubleToFloat.cpp:76 > +// Using float in place remove the useless conversion, and the float removes > Source/JavaScriptCore/b3/B3ReduceDoubleToFloat.cpp:77 > +// ops is sometime massively cheaper (SQRT for example). op