Re: [PATCH] PR 14631, common subexpression elimination error withsse2

[Scott Robert Ladd <coyote at coyotegulch dot com>]

Note to self: If my logic below is correct for PINSRW, PEXTRW should 
also be changed (from vec_select to upspec) since it, too, is not a 
vector operation.

The patch, as it stands, only addresses PINSRW, since that was the 
offending instruction in the PR. Assuming I'm not completely ignorant 
here, shouldn't I extend the patch to alter PEXTRW as well? Or should 
that be a separate patch, since it is not explicitly part of the PR?

..Scott

Scott Robert Ladd wrote:
> Richard Henderson wrote:
>
> > How does this fix the bug?  I don't see anything in the md file
> > that should be conflicting here...
>
>
> The PINSRW instruction is not a vector operation, insofar as I can tell 
> from reading AMD's documentation on SSE2 instructions (I have the Intel 
> docs, but haven't checked them on this issue.) To quote AMD:
>
> [QUOTE]
> The PINSRW instruction inserts a 16-bit value from the low-order word of 
> a general purpose register or from a 16-bit memory location into an XMM 
> register.... The other words in the destination register operand are not 
> modified.
> [QUOTE]
>
> When defined as vec_merge, the code generator incorrectly inserts PINSRW 
> into the instruction stream; by changing PINSRW to unspec, correct code 
> is generated.
>
> I've always considered the proof to be in the pudding: Without the fix, 
> mainline miscompiles the testcase; with this patch applied, mainline 
> produces a correct program. The patch also eliminates a code generation 
> bug in one of my own applications.


--
Scott Robert Ladd
site: http://www.coyotegulch.com
blog: http://chaoticcoyote.blogspot.com