RESOLVED FIXED 190751
[JSC] sub op with 0 should be optimized 
https://bugs.webkit.org/show_bug.cgi?id=190751
Summary [JSC] sub op with 0 should be optimized
Yusuke Suzuki
Reported 2018-10-19 03:18:51 PDT
[JSC] sub op with 0 should be optimized
Attachments
Patch (5.48 KB, patch)
2018-10-19 03:40 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (5.49 KB, patch)
2019-01-20 14:51 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (5.48 KB, patch)
2019-01-20 14:53 PST, Yusuke Suzuki 		mark.lam: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2018-10-19 03:40:33 PDT
Created attachment 352779 [details] Patch
Saam Barati
Comment 2 2019-01-20 13:39:52 PST
Comment on attachment 352779 [details] Patch Is this still valid?
Yusuke Suzuki
Comment 3 2019-01-20 13:48:53 PST
Comment on attachment 352779 [details] Patch Hmm, not sure since this patch is old...
Yusuke Suzuki
Comment 4 2019-01-20 14:51:20 PST
Created attachment 359652 [details] Patch
Yusuke Suzuki
Comment 5 2019-01-20 14:53:01 PST
Created attachment 359653 [details] Patch
Yusuke Suzuki
Comment 6 2019-01-20 14:53:20 PST
Yes, it is still valid. Rebased the patch.
Mark Lam
Comment 7 2019-01-21 10:08:34 PST
Comment on attachment 359653 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359653&action=review r=me > Source/JavaScriptCore/offlineasm/arm64.rb:506 > + unless operands[0] == operands[2] Why not express this as "if operands[0] != operands[2]"? > Source/JavaScriptCore/offlineasm/x86.rb:794 > + unless operands[0] == operands[2] Why not express this as "if operands[0] != operands[2]"? > Source/JavaScriptCore/offlineasm/x86.rb:801 > + $asm.puts "add#{x86Suffix(kind)} #{orderOperands(operands[0].x86Operand(kind), operands[2].x86Operand(kind))}" For completeness, you can make this line conditional on "if Immediate.new(nil, 0) != operands[0]".
Yusuke Suzuki
Comment 8 2019-01-21 12:48:54 PST
Comment on attachment 359653 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359653&action=review Thanks >> Source/JavaScriptCore/offlineasm/arm64.rb:506 >> + unless operands[0] == operands[2] > > Why not express this as "if operands[0] != operands[2]"? OK, fixed. And I also fixed the same condition in emitARM64Add. >> Source/JavaScriptCore/offlineasm/x86.rb:794 >> + unless operands[0] == operands[2] > > Why not express this as "if operands[0] != operands[2]"? Ditto. >> Source/JavaScriptCore/offlineasm/x86.rb:801 >> + $asm.puts "add#{x86Suffix(kind)} #{orderOperands(operands[0].x86Operand(kind), operands[2].x86Operand(kind))}" > > For completeness, you can make this line conditional on "if Immediate.new(nil, 0) != operands[0]". Sounds fine, fixed!
Yusuke Suzuki
Comment 9 2019-01-21 12:51:05 PST
Committed r240241: <https://trac.webkit.org/changeset/240241>
Radar WebKit Bug Importer
Comment 10 2019-01-21 12:52:27 PST
<rdar://problem/47432315>
Note You need to log in before you can comment on or make changes to this bug.