[PATCH] D75840: [ExpandMemCmp] Improve non-equality comparisons with zero.

[Clement Courbet via Phabricator via llvm-commits]

courbet marked an inline comment as done.
courbet added inline comments.


================
Comment at: llvm/lib/CodeGen/ExpandMemCmp.cpp:370-374
+  case CmpInst::ICMP_SLT:
+  case CmpInst::ICMP_SGT:
+  case CmpInst::ICMP_SLE:
+  case CmpInst::ICMP_SGE:
+    return getCompareLoadPairsForZeroRelational(BlockIndex, LoadIndex);
----------------
spatel wrote:
> This is not correct. memcmp() compares as unsigned bytes:
> http://www.cplusplus.com/reference/cstring/memcmp/
> 
> This program will show a failure compiled with -O1 vs -O0 (do we have some kind of correctness test like this in the test-suite?):
> 
> 
> ```
> #include <string.h>
> #include <stdio.h>
> 
> void wrapper(char *c, char *d) {
>   if (memcmp(c, d, 4) > 0)
>     printf(">0\n");
>   else
>     printf("<=0\n");
> }
> 
> int main() {
>   int A = 0x00000000;
>   int B = 0x00000080;
>   wrapper(&A, &B);
>   return 0;
> }
> 
> ```
Thank for the catch. I've added more tests in `4edd050c7e97` to make the issue more obvious to the eye than the `setl`/`setb` issue.

I though the test-suite benchmark was validating the result, but it's only benchmarking and throwing the result away. I'll fix the benchmarks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75840