Looks good. I thought I said that about your original patch too, but maybe I didn't. You should make sure I didn't commit it for you already.
Gabe Quoting Vince Weaver <[email protected]>: > > I didn't get any comments on this patch when I initially posted it. > I've extended the patch to fix all of the ULL issues in the file. > > The issue is shifting 1 by more than 32 without putting an ULL on the end. > > I checked the other X86 isa files, but they are all fine, it's only the > mediaop.isa file with these problems. > > I'm not sure how this conflicts with the ULL fixes for 32-bit builds that > are going on elsewhere. > > # HG changeset patch > # User Vince Weaver <[email protected]> > # Date 1257372942 18000 > # Node ID 5d1ef2e22e7ce7a25e5bbfd4c705987190a1e9d3 > # Parent cf6a2dce697bd722b0de2b84311a536155f7b439 > X86: Fix ULL issues in mediaop.isa > > The cvti2f mediaop wasn't properly casting a "1" being shifted to ULL, > so that it defaulted to 32-bits and would wrap around. > > This led to weird behavior; among other things, it meant a > 64-bit integer with bit 31 set got sign-extended when it wasn't supposed to. > > This problem was noticed because the perlbmk makerandom input was > generating negative number results. > > diff -r cf6a2dce697b src/arch/x86/isa/microops/mediaop.isa > --- a/src/arch/x86/isa/microops/mediaop.isa Wed Nov 04 00:47:12 2009 -0500 > +++ b/src/arch/x86/isa/microops/mediaop.isa Fri Nov 06 12:26:42 2009 -0500 > @@ -452,7 +452,7 @@ > if (signBit) { > if (overflow != mask(destBits - srcBits + 1)) { > if (ext & 0x1) > - picked = (1 << (destBits - 1)); > + picked = (1ULL << (destBits - 1)); > else > picked = 0; > } > @@ -480,7 +480,7 @@ > if (signBit) { > if (overflow != mask(destBits - srcBits + 1)) { > if (ext & 0x1) > - picked = (1 << (destBits - 1)); > + picked = (1ULL << (destBits - 1)); > else > picked = 0; > } > @@ -642,10 +642,10 @@ > int loIndex = (i + 0) * sizeBits; > uint64_t arg1Bits = bits(FpSrcReg1.uqw, hiIndex, loIndex); > int64_t arg1 = arg1Bits | > - (0 - (arg1Bits & (1 << (sizeBits - 1)))); > + (0 - (arg1Bits & (1ULL << (sizeBits - 1)))); > uint64_t arg2Bits = bits(FpSrcReg2.uqw, hiIndex, loIndex); > int64_t arg2 = arg2Bits | > - (0 - (arg2Bits & (1 << (sizeBits - 1)))); > + (0 - (arg2Bits & (1ULL << (sizeBits - 1)))); > uint64_t resBits; > > if (ext & 0x2) { > @@ -680,10 +680,10 @@ > int loIndex = (i + 0) * sizeBits; > uint64_t arg1Bits = bits(FpSrcReg1.uqw, hiIndex, loIndex); > int64_t arg1 = arg1Bits | > - (0 - (arg1Bits & (1 << (sizeBits - 1)))); > + (0 - (arg1Bits & (1ULL << (sizeBits - 1)))); > uint64_t arg2Bits = bits(FpSrcReg2.uqw, hiIndex, loIndex); > int64_t arg2 = arg2Bits | > - (0 - (arg2Bits & (1 << (sizeBits - 1)))); > + (0 - (arg2Bits & (1ULL << (sizeBits - 1)))); > uint64_t resBits; > > if (ext & 0x2) { > @@ -957,7 +957,7 @@ > int resSign = bits(resBits, sizeBits - 1); > if ((arg1Sign == arg2Sign) && (arg1Sign != resSign)) { > if (resSign == 0) > - resBits = (1 << (sizeBits - 1)); > + resBits = (1ULL << (sizeBits - 1)); > else > resBits = mask(sizeBits - 1); > } > @@ -996,7 +996,7 @@ > int resSign = bits(resBits, sizeBits - 1); > if ((arg1Sign == arg2Sign) && (arg1Sign != resSign)) { > if (resSign == 0) > - resBits = (1 << (sizeBits - 1)); > + resBits = (1ULL << (sizeBits - 1)); > else > resBits = mask(sizeBits - 1); > } > @@ -1032,16 +1032,16 @@ > > if (ext & 0x2) { > int64_t arg1 = arg1Bits | > - (0 - (arg1Bits & (1 << (srcBits - 1)))); > + (0 - (arg1Bits & (1ULL << (srcBits - 1)))); > int64_t arg2 = arg2Bits | > - (0 - (arg2Bits & (1 << (srcBits - 1)))); > + (0 - (arg2Bits & (1ULL << (srcBits - 1)))); > resBits = (uint64_t)(arg1 * arg2); > } else { > resBits = arg1Bits * arg2Bits; > } > > if (ext & 0x4) > - resBits += (1 << (destBits - 1)); > + resBits += (1ULL << (destBits - 1)); > > if (ext & 0x8) > resBits >>= destBits; > @@ -1142,7 +1142,7 @@ > } else { > resBits = (arg1Bits >> shiftAmt); > resBits = resBits | > - (0 - (resBits & (1 << (sizeBits - 1 - shiftAmt)))); > + (0 - (resBits & (1ULL << (sizeBits - 1 - > shiftAmt)))); > } > > result = insertBits(result, hiIndex, loIndex, resBits); > @@ -1289,7 +1289,8 @@ > int srcHiIndex = srcStart + (i + 1) * srcSizeBits - 1; > int srcLoIndex = srcStart + (i + 0) * srcSizeBits; > uint64_t argBits = bits(FpSrcReg1.uqw, srcHiIndex, > srcLoIndex); > - int64_t sArg = argBits | (0 - (argBits & (1 << > srcHiIndex))); > + > + int64_t sArg = argBits | (0 - (argBits & (1ULL << > srcHiIndex))); > double arg = sArg; > > if (destSize == 4) { > @@ -1400,10 +1401,10 @@ > int loIndex = (i + 0) * sizeBits; > uint64_t arg1Bits = bits(FpSrcReg1.uqw, hiIndex, loIndex); > int64_t arg1 = arg1Bits | > - (0 - (arg1Bits & (1 << (sizeBits - 1)))); > + (0 - (arg1Bits & (1ULL << (sizeBits - 1)))); > uint64_t arg2Bits = bits(FpSrcReg2.uqw, hiIndex, loIndex); > int64_t arg2 = arg2Bits | > - (0 - (arg2Bits & (1 << (sizeBits - 1)))); > + (0 - (arg2Bits & (1ULL << (sizeBits - 1)))); > > uint64_t resBits = 0; > if (((ext & 0x2) == 0 && arg1 == arg2) || > _______________________________________________ > m5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/m5-dev > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
