[llvm-branch-commits] [llvm-branch] r311372 - Merging r311229:

[Hans Wennborg via llvm-branch-commits]

Author: hans
Date: Mon Aug 21 13:07:38 2017
New Revision: 311372

URL: http://llvm.org/viewvc/llvm-project?rev=311372&view=rev
Log:
Merging r311229:
------------------------------------------------------------------------
r311229 | chandlerc | 2017-08-18 23:56:11 -0700 (Fri, 18 Aug 2017) | 19 lines

[Inliner] Fix a nasty bug when inlining a non-recursive trace of
a function into itself.

We tried to fix this before in r306495 but that got reverted as the
assert was actually hit.

This fixes the original bug (which we seem to have lost track of with
the revert) by blocking a second remapping when the function being
inlined is also the caller and the remapping could succeed but
erroneously.

The included test case would actually load from an inlined copy of the
alloca before this change, failing to load the stored value and
miscompiling.

Many thanks to Richard Smith for diagnosing a user miscompile to this
bug, and to Kyle for the first attempt and initial analysis and David Li
for remembering the issue and how to fix it and suggesting the patch.
I'm just stitching it together and landing it. =]
------------------------------------------------------------------------

Modified:
    llvm/branches/release_50/   (props changed)
    llvm/branches/release_50/lib/Transforms/Utils/CloneFunction.cpp
    llvm/branches/release_50/test/Transforms/Inline/recursive.ll

Propchange: llvm/branches/release_50/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Mon Aug 21 13:07:38 2017
@@ -1,3 +1,3 @@
 /llvm/branches/Apple/Pertwee:110850,110961
 /llvm/branches/type-system-rewrite:133420-134817
-/llvm/trunk:155241,308483-308484,308503,308808,308813,308847,308891,308906,308950,308963,308978,308986,309044,309071,309113,309120,309122,309140,309227,309302,309321,309323,309325,309330,309343,309353,309355,309422,309481,309483,309495,309555,309561,309594,309614,309651,309744,309758,309849,309928,309930,309945,310066,310071,310190,310240-310242,310250,310253,310262,310267,310481,310492,310498,310510,310534,310552,310604,310779,310784,310796,310842,310906,310926,310939,310979,310988,310991,311068,311087,311258
+/llvm/trunk:155241,308483-308484,308503,308808,308813,308847,308891,308906,308950,308963,308978,308986,309044,309071,309113,309120,309122,309140,309227,309302,309321,309323,309325,309330,309343,309353,309355,309422,309481,309483,309495,309555,309561,309594,309614,309651,309744,309758,309849,309928,309930,309945,310066,310071,310190,310240-310242,310250,310253,310262,310267,310481,310492,310498,310510,310534,310552,310604,310779,310784,310796,310842,310906,310926,310939,310979,310988,310991,311068,311087,311229,311258

Modified: llvm/branches/release_50/lib/Transforms/Utils/CloneFunction.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_50/lib/Transforms/Utils/CloneFunction.cpp?rev=311372&r1=311371&r2=311372&view=diff
==============================================================================
--- llvm/branches/release_50/lib/Transforms/Utils/CloneFunction.cpp (original)
+++ llvm/branches/release_50/lib/Transforms/Utils/CloneFunction.cpp Mon Aug 21 13:07:38 2017
@@ -341,8 +341,9 @@ void PruningFunctionCloner::CloneBlock(c
               SimplifyInstruction(NewInst, BB->getModule()->getDataLayout())) {
         // On the off-chance that this simplifies to an instruction in the old
         // function, map it back into the new function.
-        if (Value *MappedV = VMap.lookup(V))
-          V = MappedV;
+        if (NewFunc != OldFunc)
+          if (Value *MappedV = VMap.lookup(V))
+            V = MappedV;
 
         if (!NewInst->mayHaveSideEffects()) {
           VMap[&*II] = V;

Modified: llvm/branches/release_50/test/Transforms/Inline/recursive.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_50/test/Transforms/Inline/recursive.ll?rev=311372&r1=311371&r2=311372&view=diff
==============================================================================
--- llvm/branches/release_50/test/Transforms/Inline/recursive.ll (original)
+++ llvm/branches/release_50/test/Transforms/Inline/recursive.ll Mon Aug 21 13:07:38 2017
@@ -37,3 +37,34 @@ declare void @foo2(i8* %in)
 
 declare i32 @foo(i32 %param)
 
+; Check that when inlining a non-recursive path into a function's own body that
+; we get the re-mapping of instructions correct.
+define i32 @test_recursive_inlining_remapping(i1 %init, i8* %addr) {
+; CHECK-LABEL: define i32 @test_recursive_inlining_remapping(
+bb:
+  %n = alloca i32
+  br i1 %init, label %store, label %load
+; CHECK-NOT:     alloca
+;
+; CHECK:         %[[N:.*]] = alloca i32
+; CHECK-NEXT:    br i1 %init,
+
+store:
+  store i32 0, i32* %n
+  %cast = bitcast i32* %n to i8*
+  %v = call i32 @test_recursive_inlining_remapping(i1 false, i8* %cast)
+  ret i32 %v
+; CHECK-NOT:     call
+;
+; CHECK:         store i32 0, i32* %[[N]]
+; CHECK-NEXT:    %[[CAST:.*]] = bitcast i32* %[[N]] to i8*
+; CHECK-NEXT:    %[[INLINED_LOAD:.*]] = load i32, i32* %[[N]]
+; CHECK-NEXT:    ret i32 %[[INLINED_LOAD]]
+;
+; CHECK-NOT:     call
+
+load:
+  %castback = bitcast i8* %addr to i32*
+  %n.load = load i32, i32* %castback
+  ret i32 %n.load
+}