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

[Wei Mi via llvm-commits]

wmi added inline comments.

================
Comment at: lib/CodeGen/InlineSpiller.cpp:1397
@@ +1396,3 @@
+    LiveInterval &OrigLI = LIS.getInterval(OrigReg);
+    // set current interval for SA to compute LastSplitPoint of BB.
+    SA->setParent(&OrigLI);
----------------
echristo wrote:
> Capital 'S' at the beginning of the sentence. 
> 
> Perhaps: "Use the current interval for SA to compute a safe insertion point in the target basic block." 
Thanks. That is a better description. will fix it.

================
Comment at: lib/CodeGen/SplitKit.h:124-126
@@ -123,5 +123,5 @@
 
   /// analyze - set CurLI to the specified interval, and analyze how it may be
-  /// split.
-  void analyze(const LiveInterval *li);
+  /// split. If SkipAnalyze is true, skip the analyzeUses call.
+  void analyze(const LiveInterval *li, bool SkipAnalyze = false);
 
----------------
majnemer wrote:
> It is a little confusing to have a function called `analyze` which takes a parameter named `SkipAnalyze`.  Perhaps it would be more obvious what you are trying to achieve if the functionality that you desired from `analyze` was split out into another method.
I only need to set current liveinterval for SplitAnalysis. I creat a method SplitAnalysis::setParent for it.

================
Comment at: lib/CodeGen/SplitKit.h:124-127
@@ -123,6 +123,6 @@
 
   /// analyze - set CurLI to the specified interval, and analyze how it may be
   /// split.
   void analyze(const LiveInterval *li);
 
   /// didRepairRange() - Returns true if CurLI was invalid and has been repaired
----------------
echristo wrote:
> I'm not sure I get it after your changes. Can you explain what you're doing now with setParent in a bit more detail?
I use setParent to set SplitAnalysis::CurLi, which will be used in SplitAnalysis::computeLastSplitPoint to compute the last split point, i.e., insert point of bb.


http://reviews.llvm.org/D19884