On Apr 11, 2013, at 9:35 AM, Reid Kleckner <[email protected]> wrote: > On Tue, Apr 9, 2013 at 7:50 PM, John McCall <[email protected]> wrote: >> + // in which case we need to skip the virtual base lookup. If there were a >> + // vbtable, the first entry is a no-op entry that gives back the original >> >> Grammar: "If there is a vbtable..." > > OK. I'm confused about subjunctive tense. =/
Understandable. :) To use the subjunctive here, you'd say something like "If there were a vbtable, the first entry would be a no-op entry that gives back the original...". But you don't really want to use the subjunctive mood because it connotes that the case is counter-factual, as opposed to something that we in fact rely on for the correctness of the code we're executing. >> + llvm::BasicBlock *OriginalBB; >> + llvm::BasicBlock *SkipAdjustBB; >> >> You'll need to zero-initialize these in order to pacify compiler warnings. > > OK. There's been some debate on the list, and I wasn't sure which way to go. It's pretty straightforward. You need to zero-initialize things that are only conditionally initialized and used because you can't rely on warnings to be clever enough to detect correlated branches. You should not zero-initialize things that you expect to be initialized along every path because doing so will disable a warning that would otherwise tell you that your expectation is wrong. >> + // Add it to VBPtr. >> + llvm::Value *AdjustedBase = Builder.CreateInBoundsGEP(VBPtr, VBaseOffs); >> >> On Win64, you need to be sure to sign-extend this. (vb-offsets can be >> negative in very particular circumstances.) > > GEP sign extends the i32 implicitly, apparently. Ah, yes, you're right. > Should I commit the patch attached? Yes, this looks great, thanks! John. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
