[PATCH] [ms-cxxabi] Implement codegen for dereferencing member pointers

[John McCall]

On Apr 11, 2013, at 9:35 AM, Reid Kleckner <rnk at google.com> wrote:
> On Tue, Apr 9, 2013 at 7:50 PM, John McCall <rjmccall at apple.com> 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.