[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