Bug 151214

I think that the right story for commutativity is just to allow B3 to commute CheckAdd/CheckMul. If you are a client of B3 and you want to emit: CheckAdd(@x, @y) And you want your generator to know what @x and @y are *before* commutation, you should do: CheckAdd(@x, @y, @x, @y) We might commute the first two args since those are relevant to the math, but we won't commute the stackmap arguments.

While doing this and talking to Ben, I realized that we need to have the notion of a LateUse: a Use that happens after all relevant Def's, i.e. it appears like it's a Use on the instruction following this one. We need this to handle: - CheckAdd(@x, @x) - CheckMul(@x, @y) where the register allocator uses the same register for the result as for the inputs

Created attachment 265634 [details] work in progress

Created attachment 265661 [details] the patch

Created attachment 265662 [details] the patch

Attachment 265662 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/b3/testb3.cpp:3216: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3219: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3250: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3251: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3254: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3285: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3286: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3322: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3354: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3426: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3429: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3461: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3462: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3499: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3536: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3607: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3636: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3670: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3709: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3710: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3754: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3755: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3758: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3788: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/air/AirLiveness.h:56: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/air/AirLiveness.h:137: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/B3StackmapSpecial.h:57: The parameter name "role" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/b3/B3CheckSpecial.cpp:158: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 28 in 32 files If any of these errors are false positives, please file a bug against check-webkit-style.

I think I got the allocator bug: https://bugs.webkit.org/show_bug.cgi?id=151345

Comment on attachment 265662 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=265662&action=review r=me > Source/JavaScriptCore/b3/B3LowerToAir.cpp:573 > - > + Revert-o.

Comment on attachment 265662 [details] the patch Clearing cq so that I can land it myself.

Landed in http://trac.webkit.org/changeset/192540