[PATCH] D127766: [InstCombine] Fold memcmp of constant arrays and variable size

[Clement Courbet via Phabricator via llvm-commits]

courbet added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:1163-1168
+      // One array is a leading part of the other of equal or greater
+      // size.
+      // Fold the result to zero.  Size is assumed to be in bounds, since
+      // otherwise the call would be undefined.
+      // Setting Pos to MAX ensures that the N <= Pos test inserted below
+      // is eventually folded away.
----------------
This feels a bit convoluted. Why not something like:

```
Value *Zero = ConstantInt::get(CI->getType(), 0);

uint64_t MinSize = std::min(LStr.size(), RStr.size());
for (uint64_t Pos = 0; Pos < MinSize; ++Pos) {
  if (LStr[Pos] != RStr[Pos]) {
    Value *MaxSize = ConstantInt::get(Size->getType(), Pos);
    Value *Cmp = B.CreateICmp(ICmpInst::ICMP_ULE, Size, MaxSize);
    int IRes = LStr[Pos] < RStr[Pos] ? -1 : 1;
    Value *Res = ConstantInt::get(CI->getType(), IRes);
    return B.CreateSelect(Cmp, Zero, Res);
  }
}
// One array is a leading part of the other of equal or greater size.   
// Fold the result to zero.  Size is assumed to be in bounds, since  
// otherwise the call would be undefined. 
return  Zero;
``` 


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:1276
   ConstantInt *LenC = dyn_cast<ConstantInt>(Size);
   if (!LenC)
+    // Hande variable Size.
----------------
[nit] It feels weird to have `if(!constant) { /*handle general case*/ } /*handle constant case*/ `. Can you rewrite it as  `if(constant) { /*handle constant case*/ } /*handle general case*/ ` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127766