Instruction Combining is wrongly optimizing the following code: %tmp30 = load i64* %tmp10 ; <i64> [#uses=1] %.cast = zext i32 63 to i64 ; <i64> [#uses=1] %tmp31 = ashr i64 %tmp30, %.cast ; <i64> [#uses=1] %tmp3132 = trunc i64 %tmp31 to i32 ; <i32> [#uses=1] %tmp33 = or i32 %tmp3132, 1 ; <i32> [#uses=1] store i32 %tmp33, i32* %tmp9 %tmp34 = load i32* %tmp9 ; <i32> [#uses=1] store i32 %tmp34, i32* %retval br label %return.exitStub to store i32 -1, i32* %tmp9 store i32 -1, i32* %retval ret void
Created attachment 1001 [details] bugpoint-tooptimize.bc
The problematic function (ffmpeg/libavutil/rational.h): /** * Rational number num/den. */ typedef struct AVRational{ int num; ///< numerator int den; ///< denominator } AVRational; /** * Compare two rationals. * @param a first rational * @param b second rational * @return 0 if a==b, 1 if a>b and -1 if a<b. */ static inline int av_cmp_q(AVRational a, AVRational b){ const int64_t tmp= a.num * (int64_t)b.den - b.num * (int64_t)a.den; if(tmp) return (tmp>>63)|1; else return 0; } -------------------------------------------------------------------------- I think the comment is wrong. This function returns 0 if a==b, 1 if a>b and -0x7FFFFFFF if a<b.
Created attachment 1002 [details] fix Arithmetic left shift is not the inverse operation of arithmetic right shift. The arithmetic left shift can result in overflow. So, we should preserve the sign bit beyond making the arithmetic left shift.
The patch is close, but not quite right. The sign bit definitely does need to be demanded in some cases, but only in those cases where one of the shifted in bits is used. For example, consider: %Y = ashr i8 %X, 3 If we demand (f.e.) the low two bits from Y, then the input sign bit is not needed. However, if any of the top three bits are demanded, then the input sign bit *is* demanded. So basically, in that code, if any of the "high bits" are demanded, you should set the sign bit as demanded. If none of the high bits are demanded, the sign bit should not be demanded. Thanks again for investigating this Lauro! -Chris
Fixed. http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070604/050287.html http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070604/050288.html