Page MenuHomePhabricator

[InstCombine] Transform strrchr to memrchr for constant strings

Authored by msebor on Jun 30 2022, 3:13 PM.



This change adds a memrchr library function call emitter and simplifies the strrchr call handler to use it instead of duplicating a subset of the former function's transformations in the handler. This in turn enables transforming calls with the empty string S such as strrchr(S, C) to the test C ? S : 0. A call to memrchr when the length of the string is known is also likely to be faster than one to strrchr with the corresponding arguments.

Diff Detail

Unit TestsFailed

60,150 msx64 debian > AddressSanitizer-x86_64-linux-dynamic.TestCases::scariness_score_test.cpp
Script: -- : 'RUN: at line 4'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only -m64 -shared-libasan -O0 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/asan/TestCases/scariness_score_test.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/asan/X86_64LinuxDynamicConfig/TestCases/Output/scariness_score_test.cpp.tmp
60,070 msx64 debian > AddressSanitizer-x86_64-linux.TestCases::scariness_score_test.cpp
Script: -- : 'RUN: at line 4'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only -m64 -O0 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/asan/TestCases/scariness_score_test.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/asan/X86_64LinuxConfig/TestCases/Output/scariness_score_test.cpp.tmp

Event Timeline

msebor created this revision.Jun 30 2022, 3:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 30 2022, 3:13 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
msebor requested review of this revision.Jun 30 2022, 3:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 30 2022, 3:13 PM
nikic added inline comments.Jul 1 2022, 12:43 AM

This one probably needs an isLibFuncEmittable check, because memrchr is a GNU extension. It looks like PS targets do mark it as unavailable.

msebor added inline comments.Jul 1 2022, 8:19 AM

isLibFuncEmittable is called from emitLibCall so I think this parts should be fine. Or am I missing something here?


I did forget that memrchr isn't standard so this will fail on targets without it as you noted. Are we okay with losing that optimization on those targets or should I keep the existing optimization and use it when emitMemRChr fails?

nikic accepted this revision.Jul 1 2022, 8:33 AM



Nope, my bad. It also looks like copyFlags will already handle a null argument correctly, so your code is fine as-is.


As it's just the one target that disables it, I think it's fine to drop the optimization.

This revision is now accepted and ready to land.Jul 1 2022, 8:33 AM
This revision was landed with ongoing or failed builds.Jul 1 2022, 10:12 AM
This revision was automatically updated to reflect the committed changes.