https://bugs.kde.org/show_bug.cgi?id=356393

--- Comment #5 from Florian Krohm <flor...@eich-krohm.de> ---
This is the guilty code:

            case Iop_Xor8:
            case Iop_Xor16:
            case Iop_Xor32:
            case Iop_Xor64:
            case Iop_XorV128:
            case Iop_XorV256:    //   <==== selecting this case
               /* Xor8/16/32/64/V128(t,t) ==> 0, for some IRTemp t */
               if (sameIRExprs(env, e->Iex.Binop.arg1, e->Iex.Binop.arg2)) { 
// <=== false
                  e2 = mkZeroOfPrimopResultType(e->Iex.Binop.op);
                  break;
               }
               /* XorV128(t,0) ==> t */
               if (e->Iex.Binop.op == Iop_XorV128) {
                  if (isZeroV128(e->Iex.Binop.arg2)) {
                     e2 = e->Iex.Binop.arg1;
                     break;
                  }
                  //Disabled because there's no known test case right now.
                  //if (isZeroV128(e->Iex.Binop.arg1)) {
                  //   e2 = e->Iex.Binop.arg2;
                  //   break;
                  //}
               } else {
                  /* Xor8/16/32/64(0,t) ==> t */
                  if (isZeroU(e->Iex.Binop.arg1)) {     //  <=== arriving here

BANG. isZeroU cannot handle Iop_XorV256  hence the assert.

This patch is likely going to fix it (still testing):
Index: VEX/priv/ir_opt.c
===================================================================
--- VEX/priv/ir_opt.c    (revision 3206)
+++ VEX/priv/ir_opt.c    (working copy)
@@ -2292,8 +2292,15 @@
                   e2 = mkZeroOfPrimopResultType(e->Iex.Binop.op);
                   break;
                }
+               /* XorV256(t,0) ==> t */
+               if (e->Iex.Binop.op == Iop_XorV256) {
+                  if (isZeroV256(e->Iex.Binop.arg2)) {
+                     e2 = e->Iex.Binop.arg1;
+                     break;
+                  }
+               }
                /* XorV128(t,0) ==> t */
-               if (e->Iex.Binop.op == Iop_XorV128) {
+               else if (e->Iex.Binop.op == Iop_XorV128) {
                   if (isZeroV128(e->Iex.Binop.arg2)) {
                      e2 = e->Iex.Binop.arg1;
                      break;

but the code is UGLY. Comments are out of date and the Boolean IRops And/Or/Xor
are handled differently. We really should keep as much code in common as
possible. 
Extending isZeroU and isOneU to also handle V128 and V256 values makes sense to
me.Even if the function name then no longer exactly describes the set of
allowed values. Or we can change the function name as there are only aboout 10
invocations. So the ripple is acceptable IMHO.

-- 
You are receiving this mail because:
You are watching all bug changes.

Reply via email to