[llvm-commits] Review Request for modification to Instruction::IsIdenticalToWhenDefined

[Evan Cheng]

This is scary.  The fix looks obvious to me but I'm surprised it hasn't cause more issues. How is jump threading using this? I don't see a call to isIdenticalTo or isIdenticalToWhenDefined.

Evan

On May 9, 2012, at 4:44 PM, Joel Jones <joel_k_jones at apple.com> wrote:

> Fix a problem with incomplete equality testing of PHINodes in Instruction::IsIdenticalToWhenDefined.
> 
> This manifested itself when inlining two calls to the same function.  The inlined function had a switch statement that returned one of a set of global variables.  Without this modification, the two phi instructions that chose values from the branches of the switch instruction inlined from the callee were considered equivalent and jump-threading replaced a load for the first switch value with a phi selecting from the second switch, thereby producing incorrect code.
> 
> This patch has been tested with "make check-all", "lnt runteste nt", and llvm self-hosted, and on the original program that had this problem, wireshark.
> 
> <rdar://problem/11025519>
> 
> Attached is a test case which will reside in test/Transforms/JumpThreading
> 
> Index: lib/VMCore/Instruction.cpp
> ===================================================================
> --- lib/VMCore/Instruction.cpp	(revision 156460)
> +++ lib/VMCore/Instruction.cpp	(working copy)
> @@ -226,7 +226,15 @@
>            RMWI->isVolatile() == cast<AtomicRMWInst>(I)->isVolatile() &&
>            RMWI->getOrdering() == cast<AtomicRMWInst>(I)->getOrdering() &&
>            RMWI->getSynchScope() == cast<AtomicRMWInst>(I)->getSynchScope();
> -
> +	if (const PHINode *thisPHI = dyn_cast<PHINode>(this)) {
> +		// The check above ensures that the dyn_cast and getIncomingBlock calls
> +		// below won't fail, as the type of otherPHI must be PHINode and the
> +		// number of incoming blocks must be the same.
> +		const PHINode *otherPHI = dyn_cast<PHINode>(I);
> +		for (unsigned i = 0, e = thisPHI->getNumOperands(); i != e; ++i) 
> +		  if (thisPHI->getIncomingBlock(i) != otherPHI->getIncomingBlock(i))
> +			  return false;
> +	}
>   return true;
> }
> <phi-eq.ll>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits