On MIPS the LLInt stack frame size was only 20 bytes while code generated for jsCodeEntryFor(JSC::CodeForConstruct) uses more stack space.
Created attachment 217960 [details] proposed patch.
Comment on attachment 217960 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=217960&action=review > Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:193 > + const extraStackSpace = 28 FYI 28 seems to be not enough on my mips board, with run-fast-jsc I get: - "273 tests passed, 227 tests failed, 227 tests crashed." with extraStackSpace = 20 - "437 tests passed, 63 tests failed, 63 tests crashed." with extraStackSpace = 28 - "490 tests passed, 10 tests failed, 9 tests crashed." with extraStackSpace = 32
Comment on attachment 217960 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=217960&action=review >> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:193 >> + const extraStackSpace = 28 > > FYI 28 seems to be not enough on my mips board, with run-fast-jsc I get: > - "273 tests passed, 227 tests failed, 227 tests crashed." with extraStackSpace = 20 > - "437 tests passed, 63 tests failed, 63 tests crashed." with extraStackSpace = 28 > - "490 tests passed, 10 tests failed, 9 tests crashed." with extraStackSpace = 32 Due to alignment, I think you want to add space in amounts of 16. What are the test results with 36?
Created attachment 218110 [details] fixed patch. I found that for 8 byte alignment reasons the 20 + x * 8 formula is reliable for extraStackSpace and in tests the lowest best value is 36 (It sees we have to add space for the four parameter registers on the stack also even if they aren't used). In mozilla tests there are still 13 regressions but they are caused by invalid register gp values. I think the fix of the gp problem should go to an other bug report/patch.
(In reply to comment #3) > Due to alignment, I think you want to add space in amounts of 16. What are the test results with 36? I get the same results with 32 and 36 for layout jsc tests. (In reply to comment #4) > Created an attachment (id=218110) [details] > fixed patch. Patch looks good to me.
(In reply to comment #5) > (In reply to comment #3) > > Due to alignment, I think you want to add space in amounts of 16. What are the test results with 36? > > I get the same results with 32 and 36 for layout jsc tests. Alignment problem comes up on mips32r2 boards where value 32 breaks the 8 by alignment of the stack and causes 1066 failures while value 36 is ok so stack is aligned. > (In reply to comment #4) > > Created an attachment (id=218110) [details] [details] > > fixed patch. > > Patch looks good to me.
Comment on attachment 218110 [details] fixed patch. Clearing flags on attachment: 218110 Committed r159935: <http://trac.webkit.org/changeset/159935>
All reviewed patches have been landed. Closing bug.