Thanks! Can you please commit for me? I don't have write access to the repo.

Fixed clang-format-patch failure.

I ended up putting the assumption before the ObjectSize. Here's an update that puts the assumption on the expression we built since that's what the comment says anyway. @serge-sans-paille your method of shadowing Builder and inserting after ObjectSize would work too (I should have looked carefully the first time, sorry) but I thought this way flows better with the rest of the code in that block. I'll be happy to use your approach if you think that's better.

Updated patch. I'm not entirely sure if I 'fixed' the failing test case (limit-max-iterations.ll) correctly. Earlier it would reduce in two instcombine iterations but with this assumption, it reduces to its minimal form (i.e. with just the llvm.assume and ret) in just one iteration.

In D93015#2463046, @serge-sans-paille wrote:

@siddhesh what anout the following?

Updated version using Uses and getOperandNo as suggested. I have not used llvm.assume yet, more in another comment.

In D93015#2454403, @serge-sans-paille wrote:

When __builtin_dynamic_object_size returns a non-constant expression, it cannot be -1 since that is an invalid return value for object size

I disagree with that statement. For instance

void copy_into_buffer(char* buffer) {
   __builtin___strlcpy_chk(buffer, "some string", strlen("some string"), __builtin_dynamic_object_size(buffer, 0))
 }

In that case, __builtin_dynamic_object_size may return -1, e.g. if copy_into_buffer has external linkage, and thus no information on buffer is available

More formatting fixes, sorry for the noise.

Updated for coding style fixes.

I don't remember anymore why I needed this, so abandoning it.

I do not have commit access, so please commit the change for me. Thanks!

In D91677#2405137, @serge-sans-paille wrote:

Interestingly, gcc and clang have different behavior here: https://godbolt.org/z/hdadhY

In D91677#2403232, @lebedev.ri wrote:

And by mark them, you could also use -fno-builtin=???, see D68028.

Made the change more generic, avoiding simplification if the callee has an implementation, i.e. its body is not empty.

In D91677#2401770, @serge-sans-paille wrote:

The problem you're trying to solve doesn't seem specific to bcopy: any function with an inline definition should probably *not* be replaced, and the inlined definition should be favored. This was the spirit of that patch https://reviews.llvm.org/D71082. i wonder why your test case doesn't fall into that configuration.

Sorry, I think I ran only check-llvm. I'll look at the test breakage.

Sorry, the fix should have been in setting of baseline, not in the baseline menu itself. Fixed and tested.