[PATCH] D19884: Fix a bug when hoist spill to a BB with landingpad successor

[Wei Mi via llvm-commits]

wmi added a comment.

> As for the testcase I believe what Quentin was asking was not to create a new .cc file as a test but rather to simplify and cleanup the hoist-spill-lpad.ll according to his suggestions.


hoist-spill-lpad.ll is a new testcase generated from the .cc file. I uploaded the .cc file too for easier review. I think the 
new hoist-spill-lpad.ll testcase will be relatively stable for other compiler changes.

Thanks,
Wei.


================
Comment at: lib/CodeGen/InlineSpiller.cpp:72
@@ -70,1 +71,3 @@
 
+  // Analysis to determine the latest point in a Block to insert spill.
+  std::unique_ptr<InsertPointAnalysis> IPA;
----------------
MatzeB wrote:
> MatzeB wrote:
> > This should better be a member instead of being allocated on the heap. You can add a reset() method to InsertPointAnalysis to enable this.
> This comment doesn't contain any additional information over what the user would found in the description of the InsertPointAnalysis class. I'd leave it out.
will remove it.

================
Comment at: lib/CodeGen/InlineSpiller.cpp:72-73
@@ -70,1 +71,4 @@
 
+  // Analysis to determine the latest point in a Block to insert spill.
+  std::unique_ptr<InsertPointAnalysis> IPA;
+
----------------
wmi wrote:
> MatzeB wrote:
> > MatzeB wrote:
> > > This should better be a member instead of being allocated on the heap. You can add a reset() method to InsertPointAnalysis to enable this.
> > This comment doesn't contain any additional information over what the user would found in the description of the InsertPointAnalysis class. I'd leave it out.
> will remove it.
ok, I will make InsertPointAnalysis a member of HoistSpillHelper. reset() is not needed because the lifetimes of InsertPointAnalysis, the LastSplitPoint[] cache and HoistSpillHelper will be the same. When we only reset InsertPointAnalysis::CurLI, the cache is still valid.


Repository:
  rL LLVM

http://reviews.llvm.org/D19884