Thanks for the review. Rodolph will address the last comments and upload the patch.
Alexandre http://codereview.chromium.org/6124005/diff/1/src/arm/lithium-codegen-arm.cc File src/arm/lithium-codegen-arm.cc (right): http://codereview.chromium.org/6124005/diff/1/src/arm/lithium-codegen-arm.cc#newcode1384 src/arm/lithium-codegen-arm.cc:1384: // the temp registers, but not the input. Only input and temp2 may alias. Indeed. Changed the code to have input and temp2 always different. I have a few questions however: - The ia32 comment states that input is not trashed. Is it required? - More generally, where can I find information on what each LCodeGen stub requires? I can find some info in the lithium-<arch>.* files, but how do I know for example if the stub is allowed to trash the input? On 2011/01/10 09:56:08, Søren Gjesse wrote:
input is trashed if temp2 aliases it.
http://codereview.chromium.org/6124005/diff/1/src/arm/lithium-codegen-arm.cc#newcode1398 src/arm/lithium-codegen-arm.cc:1398: On 2011/01/10 09:56:08, Søren Gjesse wrote:
How about just
// temp: map of input // temp2: object type of input
Done. http://codereview.chromium.org/6124005/diff/1/src/arm/lithium-codegen-arm.cc#newcode1412 src/arm/lithium-codegen-arm.cc:1412: __ ldr(temp, FieldMemOperand(temp, Map::kConstructorOffset)); On 2011/01/10 09:56:08, Søren Gjesse wrote:
After this
// temp: constructor
Done. http://codereview.chromium.org/6124005/diff/1/src/arm/lithium-codegen-arm.cc#newcode1427 src/arm/lithium-codegen-arm.cc:1427: On 2011/01/10 09:56:08, Søren Gjesse wrote:
Change comment to
// Get the instance class name from the constructor.
Done. http://codereview.chromium.org/6124005/diff/1/src/arm/lithium-codegen-arm.cc#newcode1444 src/arm/lithium-codegen-arm.cc:1444: void LCodeGen::DoClassOfTest(LClassOfTest* instr) { Implemented. On 2011/01/10 09:56:08, Søren Gjesse wrote:
Maybe implement this now you have done EmitClassOfTest.
http://codereview.chromium.org/6124005/diff/1/src/arm/lithium-codegen-arm.cc#newcode1454 src/arm/lithium-codegen-arm.cc:1454: // Swap. Removed. Not needed anymore, as all these registers should be different. On 2011/01/10 09:56:08, Søren Gjesse wrote:
There is a Swap instruction in the macro assembler.
http://codereview.chromium.org/6124005/diff/1/src/arm/lithium-codegen-arm.cc#newcode1888 src/arm/lithium-codegen-arm.cc:1888: DoubleRegister input_reg = ToDoubleRegister(instr->input()); On 2011/01/10 09:56:08, Søren Gjesse wrote:
input_reg -> input?
Done. http://codereview.chromium.org/6124005/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
