Re: [Qemu-devel] Redundant repz prefixes in generated amd64 code

[Axel Zeuner <axel.zeuner@gmx.de>]

Hi,

On Saturday 17 March 2007 10:51, Johannes Schindelin wrote:
> Hi,
>
> On Sat, 17 Mar 2007, axel wrote:
> > Why there exist two different blocks for COFF and ELF for x86/x86_64
> > hosts?
>
> Because COFF is used by Windows, and ELF by Linux, and they are
> substantially different?
>
Sorry, I did not want to criticise the code, I apologise for that.
But do these blocks different things? They should check for the last byte, 
strip off trailing padding bytes including the ret instruction and determine 
the size of the block of code to copy. Am I really wrong here?
>
> > @@ -1458,6 +1458,8 @@
> >              error("empty code for %s", name);
> >          if (p_end[-1] == 0xc3) {
> >              len--;
> > +           if ( len>0 && p_end[-2] == 0xf3 )
> > +               --len;
>
> This is wrong in several accounts:
>
> - style (space after opening parentheses and before closing parentheses,
>   no space before and after ">", "--" before instead of after "len", just
>   see the if clause above)
I agree, sorry for that, next time I will follow the coding rules. Most of my 
time I use C++ and there it makes sense to prefer prefix decrement and prefix 
increment operations for performance reasons.
> - if you want to access "p_end[-2]", you must check for "len > 1"
I do not agree, because len was decremented in the line above and len is a 
signed int and p_end was not changed. 
>
> - you most likely want to check "p_end[-1]" anyway
No, because p_end[-1] was already checked and is known to be 0xc3. I want to 
check the byte before p_end[-1], because repz; ret translates to 0xf3 0xc3
>
> - worst: there is no appropriate explanation why this patch is needed, and
The currently generated op_XXX functions are not affected by the stale repz 
prefixes at the end of the generated and copied blocks, but the following 
scenario is possible, at least in theory:
op_1:
	movl $0,%%ecx
	do_what_ever_but_do_not_change_ecx
	repz; ret

op_2:
	stosd
	ret
Now the following op code sequence op_1, op_2 is generated. The resulting code 
in the code generation buffer will be
	movl $0,%%ecx
	do_what_ever_but_do_not_change_ecx
	repz; # stale from op_1
	stosd; # body of op_2
This is probably not what one wants to execute.
>   even more importantly, why it does not break existing code
I agree fully, as I mentioned, this is a HACK and WILL break existing code 
sooner or later. 

Kind regards
Axel

> Hth,
> Dscho
>
>
>
>
> _______________________________________________
> Qemu-devel mailing list
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel