[turbofan] IA: support better left operand for commutative binops

[turbofan] IA: support better left operand for commutative binops

[Weiliang, 2014-09-23 04:41:01]

weiliang.lin@intel.com changed reviewers:
+ bmeurer@chromium.org, danno@chromium.org, titzer@chromium.org

[Weiliang, 2014-09-23 04:41:01]

PTAL

[Benedikt Meurer, 2014-09-23 04:56:32]

Thinking about the CanBeBetterLeftOperand thing again...maybe instead of making
IsUsed() and IsDefined() public, we can have a public method

// Checks if {node} is currently live.
bool IsLive(Node* node) const { return !selector()->IsDefined(node) &&
selector()->IsUsed(node); }

in the InstructionSelector class, which can be used to implement your
CanBeBetterLeftOperand.

https://codereview.chromium.org/591343002/diff/1/src/compiler/ia32/instructio...
File src/compiler/ia32/instruction-selector-ia32.cc (right):

https://codereview.chromium.org/591343002/diff/1/src/compiler/ia32/instructio...
src/compiler/ia32/instruction-selector-ia32.cc:41: bool
CanBeBetterLeftOperand(Node* node) { return !selector()->IsUsed(node); }
Nit: method should be const.

[Benedikt Meurer, 2014-09-23 04:57:37]

https://codereview.chromium.org/591343002/diff/1/src/compiler/ia32/instructio...
File src/compiler/ia32/instruction-selector-ia32.cc (right):

https://codereview.chromium.org/591343002/diff/1/src/compiler/ia32/instructio...
src/compiler/ia32/instruction-selector-ia32.cc:315: Emit(kIA32Imul,
g.DefineSameAsFirst(node), g.UseRegister(left),
There should be no need to use DefineSameAsFirst with imul.

[Weiliang, 2014-09-23 05:22:12]

Good idea! Please see the patch set2.

Thanks
-Weiliang

https://codereview.chromium.org/591343002/diff/1/src/compiler/ia32/instructio...
File src/compiler/ia32/instruction-selector-ia32.cc (right):

https://codereview.chromium.org/591343002/diff/1/src/compiler/ia32/instructio...
src/compiler/ia32/instruction-selector-ia32.cc:41: bool
CanBeBetterLeftOperand(Node* node) { return !selector()->IsUsed(node); }
On 2014/09/23 04:56:31, Benedikt Meurer wrote:
> Nit: method should be const.

Done.

https://codereview.chromium.org/591343002/diff/1/src/compiler/ia32/instructio...
src/compiler/ia32/instruction-selector-ia32.cc:315: Emit(kIA32Imul,
g.DefineSameAsFirst(node), g.UseRegister(left),
On 2014/09/23 04:57:37, Benedikt Meurer wrote:
> There should be no need to use DefineSameAsFirst with imul.

My thought here is that three operand imul requires an imm input. If there is no
imm input, we can only use the two-operands/single-operand imul. That is why I
still use DefineSameAsFirst here.

[Benedikt Meurer, 2014-09-23 05:41:31]

lgtm

[Benedikt Meurer, 2014-09-25 07:43:11]

Landed manually with r24205.