Fold strcmp for short string literals with size 2.
Depends D155742.
Details
Diff Detail
Event Timeline
llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp | ||
---|---|---|
909 | I would just generalize this to something like: "Try to expand strcmp(P, string_literal) where strlen(string_literal) <= 2" | |
927 | instead of == 1 || == 2 how about just <= 2? Does zero length ever occur? If so thats probably worth handling as well. | |
1022 | This is not really easy to read. It also seems to be largerly a duplicate of the size == 1 logic. |
llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp | ||
---|---|---|
1043 | Likewise, do you need the sub at all? Can you just br on the compare of the two characters and return the compare result itself? |
llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp | ||
---|---|---|
1038 | Above we check that strcmp is only used in any comparison with zero, not just equality comparison. | |
1043 | We need sub because we want to support not only equality compare strcmp result with zero strcmp(P, 'a') == 0, but also strcmp(P, 'a') < 0 and strcmp(P, 'a') > 0. |
LGTM.
Not really familiar enough with AggressiveInstCombine to be only reviewer here so please wait at least a few days to a second signoff.
llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp | ||
---|---|---|
1048 | nit can you rename *SubZero* variables -> *SubIsZero* or *Eq* (I first read 'SubZero' as negative). |
Looks good to me as well.
llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp | ||
---|---|---|
939 | Drop braces (and maybe combine these two conditions?) |
I will fix nit picks in separate patch, where I plan to add more efficient implementations for strcmp(P, 'x') == 0, strcmp(P, 'x') != 0.
I would just generalize this to something like: "Try to expand strcmp(P, string_literal) where strlen(string_literal) <= 2"