[PATCH] D50233: [InstCombine] Transform str(n)cmp to memcmp

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 4 12:10:40 PDT 2018


efriedma added a comment.

"which would observe uninitialized data at the end of the first string"

The uninitialized data isn't a problem for any implementation of memcmp that doesn't involve LTO: a cross-translation-unit call effectively freezes all unintialized data.  But I guess we can't really guarantee that reliably unless we introduce an llvm.memcmp intrinsic.

In theory there's also a potential race condition with this transform, if the memcmp implementation loads from the buffer multiple times and the "uninitialized" data is written by another thread.  We can maybe work around this; haven't thought through what check would be necessary.

We can solve both problems, I think, if we don't actually call the libc memcmp; we can emit an inline implementation that avoids those issues, I think.  At least when msan is disabled.

If this is blocking you, you can revert for now, and we'll rework this.


Repository:
  rL LLVM

https://reviews.llvm.org/D50233





More information about the llvm-commits mailing list