[PATCH] D13623: [WinEH] Iterate state changes instead of invokes

Joseph Tremoulet via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 13 09:43:03 PDT 2015


JosephTremoulet added a comment.

Thanks for the review!


================
Comment at: lib/CodeGen/AsmPrinter/WinException.cpp:406
@@ +405,3 @@
+      const MachineInstr &MI = *MBBI;
+      if (MI.isEHLabel()) {
+        MCSymbol *Label = MI.getOperand(0).getMCSymbol();
----------------
rnk wrote:
> You can save a level of indentation by handling the MI.isCall() case before the isEHLabel() check, and then changing this check to
>   if (!MI.isEHLabel())
>     continue;
Done, thanks.

================
Comment at: lib/CodeGen/AsmPrinter/WinException.cpp:534
@@ +533,3 @@
+  MachineFunction::const_iterator End = MF->end();
+  MachineFunction::const_iterator Stop = std::next(MF->begin());
+  while (Stop != End && !Stop->isEHFuncletEntry())
----------------
rnk wrote:
> We don't actually need this std::next, the entry block should never return true for `isEHFuncletEntry`. David pointed this out to me in person last week.
I'm inclined to leave this one as is:
 - I feel like it's a bit more readable this way
 - While removing `&& MBB != MF->begin()` in the previous version would have saved some work (a useless test in each iteration of the loop), keeping the call to `std::next` above the loop in this version actually saves a tiny bit of work (one unnecessary iteration of the loop)

Happy to come back and change it if you feel strongly, though.


http://reviews.llvm.org/D13623





More information about the llvm-commits mailing list