[llvm-commits] [llvm] r156548 - in /llvm/trunk: lib/VMCore/Instruction.cpp test/Transforms/JumpThreading/phi-eq.ll

[Eli Friedman]

On Thu, May 10, 2012 at 8:59 AM, Joel Jones <joel_k_jones at apple.com> wrote:
> Author: joel_k_jones
> Date: Thu May 10 10:59:41 2012
> New Revision: 156548
>
> URL: http://llvm.org/viewvc/llvm-project?rev=156548&view=rev
> Log:
> 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>
>
>
> Added:
>    llvm/trunk/test/Transforms/JumpThreading/phi-eq.ll
> Modified:
>    llvm/trunk/lib/VMCore/Instruction.cpp
>
> Modified: llvm/trunk/lib/VMCore/Instruction.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Instruction.cpp?rev=156548&r1=156547&r2=156548&view=diff
> ==============================================================================
> --- llvm/trunk/lib/VMCore/Instruction.cpp (original)
> +++ llvm/trunk/lib/VMCore/Instruction.cpp Thu May 10 10:59:41 2012
> @@ -226,7 +226,14 @@
>            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)) {
> +    const PHINode *otherPHI = cast<PHINode>(I);
> +    for (unsigned i = 0, e = thisPHI->getNumOperands(); i != e; ++i) {
> +      if (thisPHI->getIncomingBlock(i) != otherPHI->getIncomingBlock(i))
> +        return false;
> +    }
> +    return true;
> +  }
>   return true;
>  }
>
>
> Added: llvm/trunk/test/Transforms/JumpThreading/phi-eq.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreading/phi-eq.ll?rev=156548&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/JumpThreading/phi-eq.ll (added)
> +++ llvm/trunk/test/Transforms/JumpThreading/phi-eq.ll Thu May 10 10:59:41 2012
> @@ -0,0 +1,419 @@
> +; RUN: llvm-as < %s | opt -jump-threading | llvm-dis | FileCheck %s
> +; Test whether two consecutive switches with identical structures assign the
> +; proper value to the proper variable.  This is really testing
> +; Instruction::isIdenticalToWhenDefined, as previously that function was
> +; returning true if the value part of the operands of two phis were identical,
> +; even if the incoming blocks were not.
> +; NB: this function should be pruned down more.
> +
> +%struct._GList = type { i8*, %struct._GList*, %struct._GList* }
> +%struct.filter_def = type { i8*, i8* }
> +
> + at capture_filters = external hidden global %struct._GList*, align 8
> + at display_filters = external hidden global %struct._GList*, align 8
> + at .str2 = external hidden unnamed_addr constant [10 x i8], align 1
> + at __PRETTY_FUNCTION__.copy_filter_list = external hidden unnamed_addr constant [62 x i8], align 1
> + at .str12 = external hidden unnamed_addr constant [22 x i8], align 1
> + at .str13 = external hidden unnamed_addr constant [31 x i8], align 1
> + at capture_edited_filters = external hidden global %struct._GList*, align 8
> + at display_edited_filters = external hidden global %struct._GList*, align 8
> + at __PRETTY_FUNCTION__.get_filter_list = external hidden unnamed_addr constant [44 x i8], align 1
> +
> +declare void @g_assertion_message(i8*, i8*, i32, i8*, i8*) noreturn
> +
> +declare void @g_free(i8*)
> +
> +declare %struct._GList* @g_list_first(%struct._GList*)
> +
> +declare noalias i8* @g_malloc(i64)
> +
> +define void @copy_filter_list(i32 %dest_type, i32 %src_type) nounwind uwtable ssp {
> +entry:
> +  call void @llvm.dbg.value(metadata !{i32 %dest_type}, i64 0, metadata !89), !dbg !90
> +  call void @llvm.dbg.value(metadata !{i32 %src_type}, i64 0, metadata !91), !dbg !92

Please remove the debug info from this test; it makes it longer and
more difficult to read.

-Eli