This change implements folding of memrchr calls with constant sizes of zero and one, analogously to memchr.
Details
Diff Detail
Event Timeline
llvm/test/Transforms/InstCombine/memrchr-3.ll | ||
---|---|---|
91 | nit: please also set some names to parameters to avoid %0, %1, etc.. |
llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp | ||
---|---|---|
873 | I'm not entirely sure this is correct. This is definitely incorrect for memchr() and I submitted D123665 to fix that. memrchr() is a GNU extension, so I'm not sure whether the same rules apply there. I guess one could argue that memrchr() should require a fully dereferenceable object, as otherwise the base pointer will already point outside the object. |
llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp | ||
---|---|---|
873 | As a data point, sanitizer interception for these functions is implemented as follows: https://github.com/llvm/llvm-project/blob/ee44896cf42d3360f7e0229c360258c92262a47b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc#L984-L1021 So while the interceptor does not assume full dereferenceability for memchr(), it does assume so for memrchr(). So your current implementation would be consistent with sanitizers. |
llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp | ||
---|---|---|
873 | Any sanitizer developer here? It would be great to know why there is a such differenr behaviour. |
llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp | ||
---|---|---|
873 | “ I guess one could argue that memrchr() should require a fully dereferenceable object, as otherwise the base pointer will already point outside the object” But yeah this makes sense |
llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp | ||
---|---|---|
873 | Good catch on the memchr annotation! For memrchr, as you note, the whole range [base, base + n) must be dereferenceable even if it's not necessarily always accessed. |
llvm/test/Transforms/InstCombine/memrchr-3.ll | ||
---|---|---|
91 | Sure, let me do that separately: rGb8bac957d18b. |
I'm not entirely sure this is correct. This is definitely incorrect for memchr() and I submitted D123665 to fix that. memrchr() is a GNU extension, so I'm not sure whether the same rules apply there. I guess one could argue that memrchr() should require a fully dereferenceable object, as otherwise the base pointer will already point outside the object.