[PATCH] D154725: [AggressiveInstCombine] Fold strcmp for short string literals
Maksim Kita via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 13 07:21:16 PDT 2023
kitaisreal marked 7 inline comments as done.
kitaisreal added inline comments.
================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:949
+ Value *Str1FirstCharacterValue =
+ B.CreateIntCast(B.CreateLoad(B.getInt8Ty(), Str1P), B.getInt32Ty(), true);
+ Value *Str2FirstCharacterValue = ConstantInt::get(
----------------
nikic wrote:
> Why the use of sign extension? The C standard says the comparison is performed on unsigned char, not signed char:
>
> > The sign of a nonzero value returned by the comparison functions memcmp, strcmp, and strncmp
> is determined by the sign of the difference between the values of the first pair of characters (both
> interpreted as unsigned char) that differ in the objects being compared.
I cast this `char` to `int32` so I can later sub them as signed integers. This looks simillar to standard glibc implementation https://github.com/zerovm/glibc/blob/master/string/strcmp.c#L45C15-L45C15. If we take this standard implementation and look and the IR it will be similar to mine https://godbolt.org/z/Pd9xKcnE8.
================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:992
+ if (!IsCallingConvC)
+ return false;
+
----------------
nikic wrote:
> Why is this relevant?
I take it from `SimplifyLibCalls.cpp`. Not sure if this is safe to ignore it for our transformation.
```
// Then check for known library functions.
if (TLI->getLibFunc(*Callee, Func) && isLibFuncEmittable(M, TLI, Func)) {
// We never change the calling convention.
if (!ignoreCallingConv(Func) && !IsCallingConvC)
return nullptr;
if (Value *V = optimizeStringMemoryLibCall(CI, Builder))
return V;
```
================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:1003
+ if (CI->isMustTailCall() || CI->isNoTailCall())
+ return false;
+
----------------
nikic wrote:
> Why is this relevant?
I take it from `InstCombineCalls.cpp`. Not sure if this is safe to ignore it for our transformation.
```
// Skip optimizing notail and musttail calls so
// LibCallSimplifier::optimizeCall doesn't have to preserve those invariants.
// LibCallSimplifier::optimizeCall should try to preseve tail calls though.
if (CI->isMustTailCall() || CI->isNoTailCall())
return nullptr;
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154725/new/
https://reviews.llvm.org/D154725
More information about the llvm-commits
mailing list