This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] x86: Properly decode EVEX.W in vcvt[u]si2s[sd] in 32-bit


On Mon, Sep 17, 2018 at 5:45 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 17.09.18 at 14:33, <hjl.tools@gmail.com> wrote:
>> On Mon, Sep 17, 2018 at 5:18 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 16.09.18 at 14:17, <hjl.tools@gmail.com> wrote:
>>>> On Sun, Sep 16, 2018 at 1:57 AM, Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>> "H.J. Lu" <hjl.tools@gmail.com> 09/14/18 7:47 PM >>>
>>>>>>--- /dev/null
>>>>>>+++ b/gas/testsuite/gas/i386/evex.d
>>>>>>@@ -0,0 +1,16 @@
>>>>>>+#objdump: -dw -Msuffix
>>>>>>+#name: i386 EVX insns
>>>>>>+
>>>>>>+.*: +file format .*
>>>>>>+
>>>>>>+
>>>>>>+Disassembly of section .text:
>>>>>>+
>>>>>>+0+ <_start>:
>>>>>>+ +[a-f0-9]+:  62 f1 d6 38 2a f0       vcvtsi2ssl %eax,\{rd-sae\},%xmm5,%xmm6
>>>>>>+ +[a-f0-9]+:  62 f1 d7 38 2a f0       vcvtsi2sdl %eax,\{rd-sae\},%xmm5,%xmm6
>>>>>>+ +[a-f0-9]+:  62 f1 d6 08 7b f0       vcvtusi2ssl %eax,%xmm5,%xmm6
>>>>>>+ +[a-f0-9]+:  62 f1 d7 08 7b f0       vcvtusi2sdl %eax,%xmm5,%xmm6
>>>>>>+ +[a-f0-9]+:  62 f1 d6 38 7b f0       vcvtusi2ssl
>> %eax,\{rd-sae\},%xmm5,%xmm6
>>>>>>+ +[a-f0-9]+:  62 f1 d7 38 7b f0       vcvtusi2sdl
>> %eax,\{rd-sae\},%xmm5,%xmm6
>>>>>
>>>>> Hmm, a new test demanding (according to what you've told me in earlier
>>>>> discussions) bad behavior: You've said that you don't want suffixes on newer
>>>>> insns when they're not needed. While these insns may indeed better have
>>>>> suffixes in 64-bit mode (they strictly need them only with memory operands),
>>>>> there's clearly nothing to disambiguate in 16- and 32-bit modes. May I ask
>>>>> for consistency please between what you demand for patches I submit and
>>>>> ones you commit, once again without even giving a little time for reviews?
>>>>>
>>>>
>>>> I didn't add any new instructions.  These testcases are written in .byte.
>>>> I just fixed the existing entries in disassembler.
>>>
>>> I didn't say "new instructions", but "new test": In a new test I don't think
>>> it is appropriate to record expectations (here: all of the l suffixes above)
>>> that are actually expected to not be there, but appear just because of
>>> brokenness. Since you touch the respective disassembler patterns
>>> anyway I don't really understand why you didn't make the bogus suffixes
>>> go away in one go. These instructions usefully have suffixes only in
>>
>> I am fixing a different issue.  Please feel free to submit a separate
>> patch to address this particular issue.
>
> I understand you're fixing a different issue, but while doing so you
> introduce a testcase with bogus expected output. I don't think new
> testcases should ever be added when their expectations don't
> match "good" output.

The expected disassembler output comes from the existing
disassembler.  I prefer to fix a single issue in a single commit.
Please feel free to submit a patch for disassembler, which may
touch other existing testcases.

-- 
H.J.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]