If the sprintf function is static, earlier optimization passes could optimize out the return value altogether, and make it void, which could break optimizations of this libcall that touch the return value.
I haven't looked in detail at the failure or discussion in D46285 or https://bugs.llvm.org/show_bug.cgi?id=37408 , so I might be missing something.
Why are we getting this far in the 1st place? Shouldn't we have avoided this problem in TargetLibraryInfoImpl::isValidProtoForLibFunc() by checking that the return type is wrong?
Indeed, that's also doable - I'm not familiar with this layer at all so I didn't know about that check. The existing checks for sprintf/snprintf don't check the return type, but by adding a return type check there, it also works fine.
|696 ↗||(On Diff #146320)|
Is it possible to make this check stronger:
|13 ↗||(On Diff #146320)|
I always make this suggestion for instcombine tests: it's better to use the script at utils/update_test_checks.py to auto-generate the CHECK lines than to manually write them.
Yup. Although I guess it's mostly a question of what optimizations require what details to match; for this particular optimization, it could have been made to handle the case of a void return as well (but it's not necessarily worth to).
I'm not convinced this really solves the problem correctly... I mean, obviously we shouldn't crash, but this doesn't deal with the underlying issue: once we transform the internal function, it is no longer the C library function, and that difference may not be reflected in the signature. See http://lists.llvm.org/pipermail/llvm-dev/2017-October/118578.html .
Thanks - the codebase that triggered the error seems to build fine now still with this reapplied.
Hm, indeed, there's clearly more aspects to it. I take it there was no further progress since, on the ideas presented there?
No, no progress on the question of the semantics of library functions with internal linkage. (It usually isn't a problem in practice because the functions in question are built with -fno-builtin, so nobody has really spent any time.)