<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">On Aug 2, 2013, at 2:34 AM, Timur Iskhodzhanov <<a href="mailto:timurrrr@google.com">timurrrr@google.com</a>> wrote:<br><div><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"> John,<br> Any comments on this?<br></div></blockquote></div><br><div><div>+llvm::Value *ItaniumCXXABI::BuildVirtualCall(CodeGenFunction &CGF,</div><div>+ GlobalDecl GD, llvm::Value *This,</div><div>+ llvm::Type *Ty) {</div></div><div><br></div><div>I know you’re just using an existing name here, but the function is still</div><div>misnamed: it doesn’t actually build the call, it just gets the virtual function</div><div>pointer. So, maybe getVirtualFunctionPointer?</div><div><br></div><div><div>+ llvm::Value *result =</div><div>+ CGF.Builder.CreateLoad(CGF.GetAddrOfLocalVar(getThisDecl(CGF)), "this");</div><div>+</div><div>+ const CXXMethodDecl *MD = cast<CXXMethodDecl>(CGF.CurGD.getDecl());</div><div>+ if (MD->isVirtual()) {</div><div>+ result = CGM.getCXXABI()</div><div>+ .adjustThisParameterInVirtualFunctionPrologue(CGF, CGF.CurGD, result);</div><div>+ }</div><div>+</div><div>+ getThisValue(CGF) = result;</div></div><div><br></div><div>The alloca should hold the correct value; that’s important for e.g. debug info.</div><div>It’d be nice to do this during argument emission, but if necessary you can just</div><div>store the adjusted value back into the alloca.</div><div><br></div><div><div>+ unsigned AS = cast<llvm::PointerType>(This->getType())->getAddressSpace();</div><div>+ llvm::Type *charPtrTy = CGF.Int8Ty->getPointerTo(AS),</div><div>+ *thisTy = This->getType();</div><div>+ if (ML.VBase) {</div><div>+ This = CGF.Builder.CreateBitCast(This, charPtrTy);</div><div>+ const ASTRecordLayout &DerivedLayout =</div><div>+ CGF.getContext().getASTRecordLayout(MD->getParent());</div><div>+ CharUnits VBaseOffset = DerivedLayout.getVBaseClassOffset(ML.VBase);</div><div>+ This = CGF.Builder.CreateConstGEP1_64(This, -VBaseOffset.getQuantity());</div><div>+ }</div><div>+</div><div>+ CharUnits StaticOffset = ML.VFTableOffset;</div><div>+ if (!StaticOffset.isZero()) {</div><div>+ assert(StaticOffset.isPositive());</div><div>+ This = CGF.Builder.CreateBitCast(This, charPtrTy);</div><div>+ This = CGF.Builder.CreateConstGEP1_64(This, -StaticOffset.getQuantity());</div><div>+ }</div></div><div><br></div><div>Just accumulate the constant offset locally and do one gep.</div><div><br></div><div>You should add a quick explanation of the algorithm here, covering why using</div><div>the vbase offset within the defining class is the right thing to do. (It doesn’t</div><div>need to be too long — just a sentence or two, then just refer to the comments</div><div>in the vf-table layout code for a more complete explanation.)</div><div><br></div><div>Also, this will eventually need vtordisp logic, right? Or is that done with thunks?</div><div><br></div><div><div>+ /// VBase - If nonzero, holds the virtual base which holds this method</div><div>+ /// in its vftable.</div><div>+ const CXXRecordDecl *VBase;</div><div><br></div></div><div>“nonnull” instead of “nonzero”.</div><div><br></div><div>Also, the exact definition is that this is the virtual base subobject which contains</div><div>the vf-table that the method definition is adjusted to.</div><div><br></div><div><div>+ /// Stores the method's "this" implicit argument type.</div><div>+ const CXXRecordDecl *ThisType;</div></div><div><br></div><div>You seem to be going through a lot of effort to propagate a type around that,</div><div>frankly, isn’t even accurate when virtual bases come into play. Consider just</div><div>passing it as an i8*, at least in the vbase case.</div><div><br></div><div>John.</div></body></html>