Bug 151124

Summary: B3 should be able to compile and canonicalize Mul
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, benjamin, commit-queue, ggaren, mark.lam, mhahnenb, msaboff, nrotem, oliver, saam, sam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 150279    
Attachments:
Description Flags
work in progress
none
a bit more
none
the patch ggaren: review+

Filip Pizlo
Reported 2015-11-10 18:58:03 PST
Patch forthcoming.
Attachments
work in progress (15.82 KB, patch)
2015-11-10 18:59 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
a bit more (23.19 KB, patch)
2015-11-10 19:47 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (29.87 KB, patch)
2015-11-10 21:20 PST, Filip Pizlo 		ggaren: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2015-11-10 18:59:23 PST
Created attachment 265260 [details] work in progress
Filip Pizlo
Comment 2 2015-11-10 19:47:01 PST
Created attachment 265261 [details] a bit more
Filip Pizlo
Comment 3 2015-11-10 21:20:41 PST
Created attachment 265268 [details] the patch
WebKit Commit Bot
Comment 4 2015-11-10 21:23:42 PST
Attachment 265268 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/b3/B3ConstDoubleValue.h:47: The parameter name "proc" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/b3/testb3.cpp:209: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:302: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2855: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2861: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 5 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
Geoffrey Garen
Comment 5 2015-11-10 21:32:59 PST
Comment on attachment 265268 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=265268&action=review r=me > Source/JavaScriptCore/b3/B3ReduceStrength.cpp:227 > + // Turn this: Mul(value, 1) > + // Into this: value > + if (factor == 1) { > + m_value->replaceWithIdentity(m_value->child(0)); > + break; > + } Should we do -1 here?
Filip Pizlo
Comment 6 2015-11-10 21:40:54 PST
(In reply to comment #5) > Comment on attachment 265268 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=265268&action=review > > r=me > > > Source/JavaScriptCore/b3/B3ReduceStrength.cpp:227 > > + // Turn this: Mul(value, 1) > > + // Into this: value > > + if (factor == 1) { > > + m_value->replaceWithIdentity(m_value->child(0)); > > + break; > > + } > > Should we do -1 here? Good point! We should.
Filip Pizlo
Comment 7 2015-11-11 12:35:49 PST
Landed in http://trac.webkit.org/changeset/192320
Note You need to log in before you can comment on or make changes to this bug.