On Wed, Nov 6, 2013 at 5:10 AM, David Majnemer <[email protected]>wrote:
> > From my experience, > `SkipUntil(EndKind, /*StopAtSemi=*/true, /*DontConsume=*/true);` > is more typical in clang than > `SkipUntil(EndKind, StopAtSemi | DontConsume);` > > It seems that some of the calls that you changed were previously nasty > (i.e. `SkipUntil(EndKind, true, true)`) which justifies a cleanup. > However I'm not sure we want a bitfield here. > > What is your justification? > Besides the general improved clarity in the source code (no more `true, false, false, true`-disease code), from a calling-convention perspective (at least on x86 where I'm familiar, but I'm pretty sure it holds for other architectures), passing N bools is extremely inefficient and will typically occupy as much of the register set as the passing N ints, and also require at least N instructions to load up the registers for the call and also on the callee side to read them, whereas a bitmask occupies one int's worth of registers and can be loaded efficiently with a single instruction (assuming all the parameters are constant at compile time, which I think is true in this case). Also, I don't think that our register allocator is smart enough to pack bools bitwise into a single register to decrease register pressure in the callee, and even if it were the extra code to do so would still present an overhead. So basically, it's a win-win from both a human and machine perspective. -- Sean Silva > > > ================ > Comment at: include/clang/Parse/Parser.h:751 > @@ -741,3 +750,3 @@ > /// token will ever occur, this skips to the next token, or to some > likely > - /// good stopping point. If StopAtSemi is true, skipping will stop at > a ';' > - /// character. > + /// good stopping point. If Flags has bit set at StopAtSemi, skipping > will > + /// stop at a ';' character. > ---------------- > The wording "has bit set" seems strange. > > > http://llvm-reviews.chandlerc.com/D2108 > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
