[PATCH] D54517: [CGP] Limit Complex Addressing mode by number of BasicBlocks to traverse

[John Brawn via Phabricator via llvm-commits]

john.brawn accepted this revision.
john.brawn added a comment.
This revision is now accepted and ready to land.

Looks OK as a fix for the reported bug, with one minor nitpick. Taking a look at PR39625 it looks like there's an underlying problem where we insert a load of useless placeholders, e.g. for

  declare void @otherfn()
  
  define i32 @fn(i32* %arg1, i32* %arg2) {
  entry:
    %gep1 = getelementptr i32, i32* %arg1, i32 4
    %gep2 = getelementptr i32, i32* %arg1, i32 8
    br i1 undef, label %a1, label %b1
  
  a1:
    call void @otherfn()
    br label %middle
  
  b1:
    call void @otherfn()
    br label %middle
  
  middle:
    br i1 undef, label %a2, label %b2
  
  a2:
    call void @otherfn()
    br label %end
  
  b2:
    call void @otherfn()
    br label %end
  
  end:
    %phi = phi i32* [ %gep1, %a2 ], [ %gep2, %b2 ]
    %val = load i32, i32* %phi, align 4
    ret i32 %val
  }

we insert a total of 9 placeholders, but actually we need only one and the rest get deleted later. So I think there's something that could be done to improve that, but that's no reason not to do this fix.



================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:3095
+          dbgs() << "Stop complex addressing mode due to exceed of threshold: "
+                 << TraverseOrder.size());
+      ST.destroyNewNodes(CommonType);
----------------
This should have a << "\n" on the end.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54517/new/

https://reviews.llvm.org/D54517