[llvm-dev] [PATCH] strlen -> strnlen optimization

[Davide Italiano via llvm-commits]

On Sun, Feb 7, 2016 at 1:47 AM, Mehdi Amini via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Hi,
>
> (llvm-dev to BCC)
>
> Thanks for this!
>
> Patches are supposed to be sent to llvm-commits and not llvm-dev (http://llvm.org/docs/DeveloperPolicy.html ).
> Also it seems that your patch was inline in the email and not attached? I couldn't apply it for some reason.
>
> First off, you're missing test cases for each ICMP possibility, and one for the wrapping case (see test/Transforms/InstCombine/simplify-libcalls.ll if you need examples).
>
> Also, see other comments inline below.
>
>
>> On Feb 6, 2016, at 8:05 PM, Michael McConville via llvm-dev <llvm-dev at lists.llvm.org> wrote:
>>
>> This addition converts strlen() calls to strnlen() when the result is
>> compared to a constant. For example, the following:
>>
>> strlen(s) < 5
>>
>> Becomes:
>>
>> strnlen(s, 5) < 5
>>
>> That way, we don't have to walk through the entire string. There is the
>> added overhead of maintaining a counter when using strnlen(), but I
>> thought I'd start with the general case. It may make sense to only use
>> this optimization with small constants. On the other hand, the impact of
>> the counter may be negligible in many or most cases due to
>> hardware-level optimizations.
>>
>> I often notice the idiom of comparing a packet or buffer against a
>> minimum length as a sanity check. I was surprised to see that this isn't
>> optimized, so I thought I'd give it a try.
>>
>> nlewycky on IRC seemed to think it was a good idea. I'm interested to
>> hear what others think.
>>
>> I have little C++ experience, so there are likely improvements to be
>> made to my patch. I tried to adhere to the style and conventions of the
>> surrounding code.
>>
>> This reintroduces emitStrNLen(), which was removed a couple months ago
>> in r253514. The only change is a getParent()->getParent() -->
>> getModule() conversion, which was done in emitStrLen() after
>> emitStrNLen() was removed.
>>

It was removed because it was dead. If it's still in the header it's
an oversight (my bad, sorry). I'm happy to get this in if there's a consumer.
Please submit that on phabricator to ease review.

Thanks,

--
Davide