https://bugs.kde.org/show_bug.cgi?id=523843
--- Comment #11 from Martin Cermak <[email protected]> --- Created attachment 196256 --> https://bugs.kde.org/attachment.cgi?id=196256&action=edit proposed patch (In reply to Mark Wielaard from comment #10) > (In reply to Martin Cermak from comment #9) > > OK, so the "trick" we are using is that we look at what will be pushed onto > the stack, > subtract that, then make sure the stack is 16 byte aligned (downwards), > add the stack argument usage again. So we know that after all arguments are > pushed > on the stack the stack is properly aligned. > > That does mean we need to save the stack and restore it before returning. > > You have to do this using guest instructions, since you don't know the guest > stack > pointer when doHelperCall is called. > > But you do know (the calculated) argbytes for the call. So you could save > the subtraction > and addition by checking argbytes % 16 == 0. You would still need to make > sure sp is > aligned, but then pushing the arguments will keep it aligned. > > Untested: > > /* Compute the padding: Take SP, subtract argbytes, 16-round down, > and add argbytes back. Get ready for true arg push */ > HReg tmp = newVRegI(env); > addInstr(env, mk_iMOVsd_RR(hregX86_ESP(), tmp)); > if ( argbytes % 16 != 0) > addInstr(env, X86Instr_Alu32R(Xalu_SUB, X86RMI_Imm(argbytes), tmp)); > addInstr(env, X86Instr_Alu32R(Xalu_AND, X86RMI_Imm(0xfffffff0), tmp)); > if ( argbytes % 16 != 0) > addInstr(env, X86Instr_Alu32R(Xalu_ADD, X86RMI_Imm(argbytes), tmp)); > addInstr(env, mk_iMOVsd_RR(tmp, hregX86_ESP())); I've verified earlier that the misalignment may come either from the doHelperCall() callers, or from the args being pushed onto the stack via pushArg(). See Comment #8 "BUT ...". Although we know the calculated arg byte size, we don't know if we started with an aligned address at the doHelperCall() entry. The check suggested above is almost equivalent to my alignment sanity check that I've just removed because of its performance penalty, and also because, as you say, it clutters the code. The only difference is that you use the modulo operator while I'm using a bitmask. Based on my perf tool measurements, the alignment check is more expensive than the alignment code itself (subtraction, masking, and addition). [ ... stuff deleted ... ] > I think sse4-x86 is kind of worst case because it uses a lot of helper calls. > But this doesn't look so bad. And this case (3) seems best. So lets go with > this one. Agreed, I've cleaned the code up a little bit and am attaching it as a proposed patch. It tests fine locally for me. Does it look good from your perspective? Please, review. -- You are receiving this mail because: You are watching all bug changes.
