[PATCH] D155743: [AggressiveInstCombine] Fold strcmp for short string literals with size 2

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:909
 
-/// Try to expand strcmp(P, "x") calls.
+/// Try to expand strcmp(P, "x"), strcmp(P, "xy") calls.
 static bool expandStrcmp(CallInst *CI, DominatorTree &DT, bool &MadeCFGChange) {
----------------
I would just generalize this to something like: "Try to expand strcmp(P, string_literal) where strlen(string_literal) <= 2" 


================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:927
     ConstantStr = Str2;
-  } else if (!HasStr2 && HasStr1 && Str1.size() == 1) {
+  } else if (!HasStr2 && HasStr1 && (Str1.size() == 1 || Str1.size() == 2)) {
     NonConstantP = Str2P;
----------------
instead of  == 1 || == 2 how about just <= 2? Does zero length ever occur? If so thats probably worth handling as well.


================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:1022
+                       StrcmpExpandSubJoinBB}});
+  }
 
----------------
This is not really easy to read. It also seems to be largerly a duplicate of the size == 1 logic.
Can you hoist it to a helper that performs the check for a given char index?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155743/new/

https://reviews.llvm.org/D155743