[AggressiveInstCombine] Memchr inline #130525
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,10 +54,9 @@ static cl::opt<unsigned> StrNCmpInlineThreshold( | |
| cl::desc("The maximum length of a constant string for a builtin string cmp " | ||
| "call eligible for inlining. The default value is 3.")); | ||
|
|
||
| static cl::opt<unsigned> MemChrInlineThreshold( | ||
| "memchr-inline-threshold", cl::init(6), cl::Hidden, | ||
| cl::desc("Size threshold for inlining memchr calls")); | ||
|
|
||
| /// Match a pattern for a bitwise funnel/rotate operation that partially guards | ||
| /// against undefined behavior by branching around the funnel-shift/rotation | ||
|
|
@@ -1106,79 +1105,46 @@ void StrNCmpInliner::inlineCompare(Value *LHS, StringRef RHS, uint64_t N, | |
| } | ||
| } | ||
|
|
||
| static bool foldMemChr(CallInst *Call, DomTreeUpdater *DTU, | ||
| const DataLayout &DL) { | ||
| Value *Ptr = Call->getArgOperand(0); | ||
| Value *Val = Call->getArgOperand(1); | ||
| Value *Len = Call->getArgOperand(2); | ||
|
|
||
| // If length is not a constant, we can't do the optimization | ||
| auto *LenC = dyn_cast<ConstantInt>(Len); | ||
| if (!LenC) | ||
| return false; | ||
|
|
||
| uint64_t Length = LenC->getZExtValue(); | ||
|
|
||
| // Check if this is a small memchr we should inline | ||
| if (Length <= MemChrInlineThreshold) { | ||
| IRBuilder<> IRB(Call); | ||
|
|
||
| // Truncate the search value to i8 | ||
| Value *ByteVal = IRB.CreateTrunc(Val, IRB.getInt8Ty()); | ||
|
|
||
| // Initialize result to null | ||
| Value *Result = ConstantPointerNull::get(cast<PointerType>(Call->getType())); | ||
|
|
||
| // For each byte up to Length | ||
| for (unsigned i = 0; i < Length; i++) { | ||
| Value *CurPtr = i == 0 ? Ptr : | ||
| IRB.CreateGEP(IRB.getInt8Ty(), Ptr, | ||
| ConstantInt::get(DL.getIndexType(Call->getType()), i)); | ||
| Value *CurByte = IRB.CreateLoad(IRB.getInt8Ty(), CurPtr); | ||
|
Member
There was a problem hiding this comment. I think it is dangerous to put these load instructions in a basic block. It is well-defined because it returns at the first occurrence. To avoid UB, we should transform this call into a if chain. BTW, your implementation returns the last occurrence of ch :( |
||
| Value *CmpRes = IRB.CreateICmpEQ(CurByte, ByteVal); | ||
| Result = IRB.CreateSelect(CmpRes, CurPtr, Result); | ||
| } | ||
|
|
||
| // Replace the call with our expanded version | ||
| Call->replaceAllUsesWith(Result); | ||
| Call->eraseFromParent(); | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| static bool foldLibCalls(Instruction &I, TargetTransformInfo &TTI, | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5 | ||
| ; RUN: opt -S -passes=aggressive-instcombine --memchr-inline-threshold=2 < %s | FileCheck %s | ||
|
|
||
| declare ptr @memchr(ptr, i32, i64) | ||
|
|
||
| define ptr @test_memchr_small(ptr %p, i32 %val) { | ||
| ; CHECK-LABEL: define ptr @test_memchr_small( | ||
| ; CHECK-SAME: ptr [[P:%.*]], i32 [[VAL:%.*]]) { | ||
| ; CHECK-NEXT: [[ENTRY:.*:]] | ||
| ; CHECK-NEXT: [[TMP0:%.*]] = trunc i32 [[VAL]] to i8 | ||
| ; CHECK-NEXT: [[TMP1:%.*]] = load i8, ptr [[P]], align 1 | ||
| ; CHECK-NEXT: [[TMP2:%.*]] = icmp eq i8 [[TMP1]], [[TMP0]] | ||
| ; CHECK-NEXT: [[TMP3:%.*]] = select i1 [[TMP2]], ptr [[P]], ptr null | ||
| ; CHECK-NEXT: [[TMP4:%.*]] = getelementptr i8, ptr [[P]], i64 1 | ||
| ; CHECK-NEXT: [[TMP5:%.*]] = load i8, ptr [[TMP4]], align 1 | ||
| ; CHECK-NEXT: [[TMP6:%.*]] = icmp eq i8 [[TMP5]], [[TMP0]] | ||
| ; CHECK-NEXT: [[TMP7:%.*]] = select i1 [[TMP6]], ptr [[TMP4]], ptr [[TMP3]] | ||
| ; CHECK-NEXT: ret ptr [[TMP7]] | ||
| ; | ||
| entry: | ||
| %res = call ptr @memchr(ptr %p, i32 %val, i64 2) | ||
| ret ptr %res | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
What is the purpose of increasing this threshold to 6?
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.
I initially set the threshold to 6 to match the test cases in memchr.ll, which test the inlining optimization for lengths up to 5 bytes. Would you recommend keeping it at 3 to be more conservative?
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.
I suggest keeping it at 3 if you do not have some performance data on llvm-test-suite/SPEC.