Created attachment 58913 [details] Patch to fix this bug There is a patch to add support for IBM POWER7 instructions in bug # 267630. This patch adds support for many new VSX instructions available with the POWER7. However, if a 32-bit app uses the xsnmaddadp instruction and you try to run that app under Valgrind's memcheck, you get the following error: ----------------- iselInt64Expr(ppc): No such tag(86023) Not64(t115) vex: the `impossible' happened: iselInt64Expr(ppc) ----------------- The attached patch fixes this problem. NOTE: This bug is dependent on bug # 267630. How can I add that dependency? I don't see an editable "Depends on" field in KDE bug tracker.
Created attachment 59142 [details] Version 2 of the patch to fix this bug
(In reply to comment #1) > Created an attachment (id=59142) [details] That looks like a version of the patch for bug 270851. For sure it doesn't look anything like the patch you attached in comment #0.
(In reply to comment #2) > (In reply to comment #1) > > Created an attachment (id=59142) [details] [details] > > That looks like a version of the patch for bug 270851. For sure > it doesn't look anything like the patch you attached in > comment #0. You're right. See my comment in 270851 -- https://bugs.kde.org/show_bug.cgi?id=270851#c5. I mistakenly attached the patch to the wrong bug. I'll make this second attachment obsolete. Attachment #1 [details] is still the patch that fixes the problem reported in this bug.
Patch generates Pun_NEG, not Pun_NOT. Did you test it?
(In reply to comment #4) > Patch generates Pun_NEG, not Pun_NOT. Did you test it? Bone-headed copy/paste error. Sorry. Yes, I tested it using memcheck, and it did get rid of the error, but evidently that was not a sufficient test. No ppc64 testcases use that code, AFAICS. There are some insns that are 64-bit category only which use Iop_Not64, but nothing where that Iop would be used in 32-bit mode. I'll attach an updated patch.
Created attachment 59401 [details] Corrected patch to fix this bug Here's the corrected patch. I tested this by adding a temporary ugly hack into guest_ppc_toIR.c (in dis_int_logic) to force the implementation for 'andc' to use Iop_Not64. Then I ran the 32-bit 'jm-insns -i' testcase (from none/test/ppc32) which has a test for 'andc', and I visually inspected the results of this hacked-up andc to make sure the Iop_Not64 did what I expect. The test hack is below: ------------------ case 0x03C: // andc (AND with Complement, PPC32 p357) DIP("andc%s r%u,r%u,r%u\n", flag_rC ? ".":"", rA_addr, rS_addr, rB_addr); if (rA_addr != 17) { assign(rA, binop( mkSzOp(ty, Iop_And8), mkexpr(rS), unop( mkSzOp(ty, Iop_Not8), mkexpr(rB)))); } else { IRTemp t1 = newTemp(Ity_I32); assign(t1, unop(Iop_64to32, unop(Iop_Not64, mkU64(0x5555333377771111ULL)))); assign(rA, mkexpr(t1)); } break; ----------------------------------- As you can see, I only do the Iop_Not64 stuff when the RA reg number is 17, because that's the output reg number used in the jm-insns testcase. Since the end result is putting the NOT of 0x77771111 into RA, what I expect printed out by the testcase is 0x8888EEEE, and that's what I see. So I'm confident that the attached patch is good. Thanks.
Committed, r2136, w/minor renaming. I also added an assert to check that Iop_Not64 is never handled by iselWordExpr_R when we're generating 32 bit code. That's untested though, so watch out for any failures (I'm pretty sure it's safe).
(In reply to comment #6) > by the testcase is 0x8888EEEE, and that's what I see. So I'm confident that > the attached patch is good. Thanks. Thanks for the extra testing -- QA, we like QA :-) In hindsight, generating NEG rather than NOT probably appeared to work most of the time because NEG mostly will simply invert bits, just like NOT, since NEG(x) == 1 + NOT(x). Or something like that.
I verified the fix in SVN head. Thanks!