Bug 133764

Summary: make sdiv a template on armv7s to remove duplicate code in dfg
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: JavaScriptCoreAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, dbates, fpizlo, ggaren
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch dbates: review+

Alex Christensen
Reported 2014-06-11 14:07:44 PDT
While playing with the css jit I noticed that sdiv is very similar on APPLE_ARMV7S and ARM64. Why not unite this code?
Attachments
Patch (5.67 KB, patch)
2014-06-11 14:08 PDT, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Patch (7.13 KB, patch)
2014-06-13 14:02 PDT, Alex Christensen 		dbates: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Alex Christensen
Comment 1 2014-06-11 14:08:53 PDT
Created attachment 232902 [details] Patch
Alex Christensen
Comment 2 2014-06-13 14:02:39 PDT
Created attachment 233074 [details] Patch
Daniel Bates
Comment 3 2014-06-18 09:27:52 PDT
Comment on attachment 233074 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=233074&action=review > Source/JavaScriptCore/assembler/ARMv7Assembler.h:1520 > + COMPILE_ASSERT(datasize == 32); Does this compile? I mean, COMPILE_ASSERT() takes two arguments: a predicate and a error message to be shown if the predicate evaluates to false. Moreover, the blog post <https://www.webkit.org/blog/3172/webkit-and-cxx11/> implies that we should explicitly use static_assert() in new C++ code instead of the COMPILE_ASSERT() macro function (even though it calls through to static_assert()) since we require ports to build WebKit with compilers that supports the C++11 static_assert feature.
Alex Christensen
Comment 4 2014-06-18 10:36:51 PDT
(In reply to comment #3) I'll use static_assert and double check that it compiles.
Alex Christensen
Comment 5 2014-06-18 11:06:51 PDT
http://trac.webkit.org/changeset/170107
Note You need to log in before you can comment on or make changes to this bug.