Bug 256669 - Unhandled loopnel insn on amd64
Summary: Unhandled loopnel insn on amd64
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: vex (show other bugs)
Version: 3.6.0
Platform: Unlisted Binaries Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks: 253451
  Show dependency treegraph
 
Reported: 2010-11-12 10:57 UTC by Jakub Jelinek
Modified: 2011-01-21 22:01 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
valgrind-3.6.0-amd64-loopnel.patch (6.87 KB, patch)
2010-11-12 10:57 UTC, Jakub Jelinek
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelinek 2010-11-12 10:57:43 UTC
Created attachment 53356 [details]
valgrind-3.6.0-amd64-loopnel.patch

Version:           3.6.0 (using Devel) 
OS:                Linux

See attached patch, loopnel insn is not handled.

Reproducible: Always
Comment 1 Julian Seward 2011-01-17 12:48:32 UTC
JJ, thanks for the patch.  One question: what is the use case
for LOOPNEL ?  Where did you see it?
Comment 2 Jakub Jelinek 2011-01-17 12:58:47 UTC
It was reported to us that some customer's application was using that instruction and valgrind was upset on it.  As the insn is valid, I think it makes sense supporting it in valgrind eventhough it isn't widely used.
Comment 3 Julian Seward 2011-01-17 15:05:39 UTC
Hmm, are you sure the patch is correct?  With 0x67 you do a 
32-bit dec (iow, ecx), fine, but the "jump if count != 0" check
is still on the 64-bit value.
Comment 4 Jakub Jelinek 2011-01-20 15:18:17 UTC
Doesn't the 32-bit dec (iow, ecx) automatically zero-extend (as is normal on x86-64 architecture and really needed for the insn too)?

The testcase certainly tests that with 0x200000005UL initial %rcx value
the loop iterates just 5 times and the final value of rcx is 0 and rax has been incremented exactly 5 times.
Comment 5 Julian Seward 2011-01-21 22:01:33 UTC
Committed with extra comments, r2085/11507.  Thanks for the patch.