RESOLVED FIXED 127709
FTL ArithMod case needs to be factored for hackability and correctness rather than just minimizing the amount of code 
https://bugs.webkit.org/show_bug.cgi?id=127709
Summary FTL ArithMod case needs to be factored for hackability and correctness rather...
Filip Pizlo
Reported 2014-01-27 14:24:45 PST
The refactoring to make ArithDiv and ArithMod share code created two bugs. One was fixed some time ago and I don't want to fix the other one. The issue here is that although the two operations appear at first to have the same lowering, they actually have subtle differences. Trying to make them share code will cause bugs. Some of those bugs will be correctness bugs and then maybe you will catch them, but other times they will be performance bugs - like a 2x regression in audio-oscillator - and then maybe you'll overlook them. The correct solution is to roll out the offending fix and to be more careful about changing ArithMod in the future. It has historically been the case that everytime someone tries to refactor ArithMod lowering, they introduce bugs. Sometimes those bugs are really hard to track down. I'm really tired of having to fix ArithMod bugs.
Attachments
the patch (10.14 KB, patch)
2014-01-27 14:25 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2014-01-27 14:25:51 PST
Created attachment 222357 [details] the patch Already rs=markH.
Filip Pizlo
Comment 2 2014-01-27 14:33:01 PST
Landed in http://trac.webkit.org/changeset/162868
Note You need to log in before you can comment on or make changes to this bug.