Re: RTL representation of i386 shrdl instruction is incorrect?

[Niranjan Hasabnis <nhasabni at cs dot stonybrook dot edu>]

Hi Richard,

Thanks for your reply. I looked into some of the details of how that
particular RTL template is used. It seems to me that the particular
RTL template is used only when shifting 64-bit data type on a 32-bit
machine. This is the underlying assumption encoded in i386.c file
which generates that particular RTL only when instruction mode is
DImode. If that is the case, then it won't matter whether one uses
arithmetic shift or logical shift to right shift lower 4-bytes of a 8-byte
value. In other words, the mapping between RTL template and shrdl
is incorrect, but the underlying assumption in i386.c guards the bug.

On Thu, Jun 5, 2014 at 3:51 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Thu, Jun 5, 2014 at 12:03 AM, Niranjan Hasabnis
> <nhasabni@cs.stonybrook.edu> wrote:
>> Hello,
>>
>> I was studying i386 machine description for my research purpose,
>> and I stumbled upon following MD entry for 'shrdl' x86 instruction.
>> It is obtained from the most recent i386.md file.
>>
>> (define_insn "x86_shrd"
>>   [(set (match_operand:SI 0 "nonimmediate_operand" "+r*m")
>>         (ior:SI (ashiftrt:SI (match_dup 0)
>>  (match_operand:QI 2 "nonmemory_operand" "Ic"))
>> (ashift:SI (match_operand:SI 1 "register_operand" "r")
>>  (minus:QI (const_int 32) (match_dup 2)))))
>>    (clobber (reg:CC FLAGS_REG))]
>>   ""
>>   "shrd{l}\t{%s2%1, %0|%0, %1, %2}"
>>   [(set_attr "type" "ishift")
>>    (set_attr "prefix_0f" "1")
>>    (set_attr "mode" "SI")
>>    (set_attr "pent_pair" "np")
>>    (set_attr "athlon_decode" "vector")
>>    (set_attr "amdfam10_decode" "vector")
>>    (set_attr "bdver1_decode" "vector")])
>>
>> It seems to me that the RTL representation for 'shrdl' is incorrect.
>>
>> Semantics of shrdl instruction as per Intel manual is:
>> "The instruction shifts the first operand (destination operand) to the right
>> the number of bits specified by the third operand (count operand).
>> The second operand (source operand) provides bits to shift in from the
>> left (starting with the most significant bit of the destination operand)."
>> And the way RTL does it is by inclusive-or of arithmetically
>> right-shifted destination and left-shifted source operand.
>>
>> But the problem is that: in case of a destination (reg/mem) containing
>> negative value, arithmetically right-shifted destination will have top bits
>> set to 1. Inclusive-or with such a value is going to generate a
>> result with top bits set to 1 instead of moving contents of source
>> into top bits of destination.
>>
>> E.g., when ebx = b72f60d0, ebp = bfcbd2c8
>>     shrdl $16, %ebp, %ebx (ebx is dest, ebp is src)
>> produces 0xd2c8b72f in ebx.
>> But the corresponding RTL produces 0xffffb72f in ebx.
>>
>> So it seems to me that instead of 'ashiftrt', RTL should have 'lshiftrt'.
>> Can anyone help me with this confusion?
>
> The way I read your explanation you are correct.  It should be possible
> to write a testcase that is miscompiled - just try to produce the
> matched RTL pattern in C and feed it with operands at runtime that
> end up producing a bogus value when shrdl is used.
>
> Oh, and you might want to file a bugreport then ;)
>
> Richard.
>
>> --
>>
>> Thanks,
>> Niranjan Hasabnis,
>> PhD student,
>> Secure Systems Lab,
>> Stony brook University,
>> Stony brook, NY.



-- 

Thanks,
Niranjan Hasabnis,
PhD student,
Secure Systems Lab,
Stony brook University,
Stony brook, NY.