On Dec 4, 2006, at 10:33 AM, Reid Spencer wrote: > On Mon, 2006-12-04 at 10:22 -0800, Chris Lattner wrote: >>> // If this is another cast that can be eliminated, it isn't >>> codegen either. >>> if (const CastInst *CI = dyn_cast<CastInst>(V)) >>> - if (isEliminableCastPair(CI, CastInst::getCastOpcode(V, Ty), >>> Ty, TD)) >>> + if (isEliminableCastPair(CI, CastInst::getCastOpcode( >>> + V, V->getType()->isSigned(), Ty, Ty->isSigned()), Ty, >>> TD)) >>> return false; >>> return true; >>> } >> >> Won't this miscompile stuff like: >> >> %B = zext sbyte %A to ushort >> %C = zext ushort %B to uint >> >> into a sext? > > No. This patch doesn't change anything.
I never claimed it wasn't broken before. We have *known* breakage in the tree still from your cast patch that we're still trying to track down. > The only reason for this change > is the change in signature for getCastOpcode. See the corresponding > change in getCastOpcode. This passed everything on Linux last night > before I committed it. Okay, this code isn't miscompiled because it isn't called in this case. After looking at it closer, I still don't see why it's safe. Specifically, why do you think it is safe for the sign of Ty or V to determine whether the cast is a sext or zext? The only way to fix this is for clients to pass in the kind of cast it is interested in. Looking at the callers, the only possibilities are sext/zext/trunc/bitcast. Treating zext and sext incorrectly *can* miscompile code. -Chris _______________________________________________ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
