-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[PowerPC][AIX] Using milicode for memcmp instead of libcall #147093
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
a46d818
cc21f3a
d835a24
97837a2
26e6c75
0d8e632
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -8820,6 +8820,45 @@ static void checkAddrSpaceIsValidForLibcall(const TargetLowering *TLI, | |||||||||
Twine(AS)); | ||||||||||
} | ||||||||||
} | ||||||||||
std::pair<SDValue, SDValue> | ||||||||||
SelectionDAG::getMemcmp(SDValue Chain, const SDLoc &dl, SDValue Mem0, | ||||||||||
SDValue Mem1, SDValue Size, const CallInst *CI) { | ||||||||||
|
||||||||||
const char *LibCallName = TLI->getLibcallName(RTLIB::MEMCMP); | ||||||||||
if (LibCallName == nullptr) | ||||||||||
return std::make_pair(SDValue(), SDValue()); | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
// Emit a library call. | ||||||||||
TargetLowering::ArgListTy Args; | ||||||||||
TargetLowering::ArgListEntry Entry; | ||||||||||
Entry.Ty = PointerType::getUnqual(*getContext()); | ||||||||||
Entry.Node = Mem0; | ||||||||||
Args.push_back(Entry); | ||||||||||
Entry.Node = Mem1; | ||||||||||
Args.push_back(Entry); | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you construct Args all at once with an initializer list |
||||||||||
|
||||||||||
Entry.Ty = getDataLayout().getIntPtrType(*getContext()); | ||||||||||
Entry.Node = Size; | ||||||||||
Args.push_back(Entry); | ||||||||||
|
||||||||||
// FIXME: pass in SDLoc | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You have the setDebugLoc below? |
||||||||||
TargetLowering::CallLoweringInfo CLI(*this); | ||||||||||
bool IsTailCall = false; | ||||||||||
bool ReturnsFirstArg = CI && funcReturnsFirstArgOfCall(*CI); | ||||||||||
IsTailCall = CI && CI->isTailCall() && | ||||||||||
isInTailCallPosition(*CI, getTarget(), ReturnsFirstArg); | ||||||||||
|
||||||||||
CLI.setDebugLoc(dl) | ||||||||||
.setChain(Chain) | ||||||||||
.setLibCallee( | ||||||||||
TLI->getLibcallCallingConv(RTLIB::MEMCMP), | ||||||||||
Type::getInt32Ty(*getContext()), | ||||||||||
getExternalSymbol(LibCallName, TLI->getPointerTy(getDataLayout())), | ||||||||||
std::move(Args)) | ||||||||||
.setTailCall(IsTailCall); | ||||||||||
|
||||||||||
std::pair<SDValue, SDValue> CallResult = TLI->LowerCallTo(CLI); | ||||||||||
return CallResult; | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
} | ||||||||||
|
||||||||||
SDValue SelectionDAG::getMemcpy( | ||||||||||
SDValue Chain, const SDLoc &dl, SDValue Dst, SDValue Src, SDValue Size, | ||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,7 +39,7 @@ define noundef i32 @_Z11memcmp_testPKvS0_m(ptr noundef readonly captures(none) % | |
; CHECK-AIX-64-P9-NEXT: mflr r0 | ||
; CHECK-AIX-64-P9-NEXT: stdu r1, -112(r1) | ||
; CHECK-AIX-64-P9-NEXT: std r0, 128(r1) | ||
; CHECK-AIX-64-P9-NEXT: bl .memcmp[PR] | ||
; CHECK-AIX-64-P9-NEXT: bl .___memcmp64[PR] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't look like a tail call? Make sure both tail and non-tail cases are tested? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
you are correct, the IR in the test case is tail call, but the generated asm is not tail call, I think it codegen issue of AIX which not related to the patch. there is a fix patch ([PowerPC] Initial support of tail call optimization on AIX for it. but not land yet. |
||
; CHECK-AIX-64-P9-NEXT: nop | ||
; CHECK-AIX-64-P9-NEXT: addi r1, r1, 112 | ||
; CHECK-AIX-64-P9-NEXT: ld r0, 16(r1) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.