https://bugs.kde.org/show_bug.cgi?id=517144
--- Comment #3 from Mark Wielaard <[email protected]> --- (In reply to Alexandra Hajkova from comment #2) > (In reply to Mark Wielaard from comment #1) > > Created attachment 190398 [details] > > Add PACKUSDW SSE4.1 support for x86 > > if ( epartIsReg(modrm) ) { > + assign( argL, getXMMReg( eregOfRM(modrm) ) ); > + delta += 2; > > I think this is wrong and it should be delta += 3 + 1 --> 3 is for skipping > 3 opcode bytes > > + DIP( "packusdw %s,%s\n", > + nameXMMReg( eregOfRM(modrm) ), > + nameXMMReg( gregOfRM(modrm) ) ); > + } else { > + addr = disAMode( &alen, sorb, delta+3, dis_buf ); > + gen_SEGV_if_not_16_aligned( addr ); > + assign( argL, loadLE( Ity_V128, mkexpr(addr) )); > + delta += 2*alen; > > I think it should be delta += 3 + alen, 3 for skipping opcode bytes You are right in both cases. I don't know what I was thinking. Also it took me a long time to see why it "worked". Turns out it was just "luck". Valgrind just disassembled the "next" instruction half way through and that instruction was "harmless" (for our actual test code). Other typos/bad delta increases do cause trouble. > + DIP( "packusdw %s,%s\n", > + dis_buf, nameXMMReg( gregOfRM(modrm) ) ); > + } > > > void test_PACKUSDW ( void ) > > I think we want 'static void' here because the test is being moved to the > header. Yes. > The patch does not apply cleanly, probably needs rebase. Looks great > otherwise. Rebased and added the above cleanups. -- You are receiving this mail because: You are watching all bug changes.
