[CIR] Implement non-virtual thunk generation for multiple inheritance #2029
Conversation
lanza
requested review from
andykaylor,
bcardosolopes and
xlauko
as code owners
| } | ||
|
|
||
| if (ShouldXRayInstrumentFunction()) { | ||
| if (d && ShouldXRayInstrumentFunction()) { |
| assert(builder.getInsertionBlock() && "Should be valid"); | ||
|
|
||
| auto fnEndLoc = getLoc(fd->getBody()->getEndLoc()); | ||
| auto fnEndLoc = (fd && fd->getBody()) ? getLoc(fd->getBody()->getEndLoc()) |
There was a problem hiding this comment.
These several extra changes on locations don't seem necessary (though probably good to have) and are very distracting
| func.setGlobalVisibilityAttr(getGlobalVisibilityAttrFromDecl(decl)); | ||
| getTargetCIRGenInfo().setTargetAttributes(funcDecl, func, *this); | ||
|
|
||
| if (const auto *_ = funcDecl->getAttr<CodeSegAttr>()) |
There was a problem hiding this comment.
Should both of these NYI assertions be "assert on missing feature" instead? or not handling them cause horrible miscompilations?
There was a problem hiding this comment.
Seems unnecessary? We need missing feature functionality when the control requires that we can enter a region that requires it and we're just exluding the feature. Both branches here we have never executed in clangir and it's safe to ignore.
| assert(!cir::MissingFeatures::ABIArgInfo()); | ||
| const CIRGenFunctionInfo &callFnInfo = CGM.getTypes().arrangeCXXMethodCall( | ||
| callArgs, fpt, RequiredArgs::forPrototypePlus(fpt, 1), prefixArgs); | ||
| // assert(callFnInfo.getRegParm() == CurFnInfo->getRegParm() && |
There was a problem hiding this comment.
What's the story with these commented pieces?
There was a problem hiding this comment.
This is actually just my preference. I prefer leaving extra detail about what we're currently missing if possible.
Stack from ghstack (oldest at bottom):
This patch implements full support for generating non-virtual thunks in
ClangIR to handle C++ multiple inheritance scenarios where the same virtual
function must be accessible through different base class pointers with
different this-pointer offsets.