[PATCH] D123198: [LibCalls] Add argument extension attributes to more functions.

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa updated this revision to Diff 426015.
jonpa added a comment.

Thanks for review.

I painstakingly went through SimplifyLibCalls and made a list of all emitted libcalls with integer arguments, and found that indeed the emission of stpncpy() was missing (but no others (!). The emission of stpncpy() never had a test, but I added one in addition to the needed recognition in getOrInsertLibFunc().

> When we see an alias, there are two options: we fail the transform (getOrInsertLibFunc returns null or something like that), or we return the attributes the caller needs to attach.

It seems most reasonable to not have the optimizers emit a call to a library function that is in fact not available. So I added a new function isLibCallEmittable() that can be used instead of TLI->has(): it also checks for an existing alias of the function. I also added an assert in getOrInsertLibFunc() that getOrInsertFunction() actually returns a Function.

I have tried to make all callers of getOrInsertLibFunc() first call isLibFuncEmittable() before emitting a libcall, which is required.

Also replaced getOrInsertFunction() with getOrInsertLibFunc() in few more places in SimplifyLibCalls.cpp

In a way this is still kind of a "mixed policy": We don't stop a libcall emission in the presence of a user defined function, but an alias however disables it. A function coming from the front end should have mandatory attributes already, so it should hopefully be ok. I was hoping the same should be true for a function alias, but IIUC the previous review that is not the case (due to incomplete function prototypes?). As this is after all the users responsibility, I don't think there has to be a full consistency and also that this is a separate topic that could perhaps be improved upon in the future if desired. There could for instance be a check that the Function seen in getOrInsertLibFunc() is a declaration with the proper libfunc arguments, and not a definition (unless the opposite handling would be done to instead try to do a best effort to add mandatory attributes to any user definitions and proceed with the libcall emission).

What do you think?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123198/new/

https://reviews.llvm.org/D123198

Files:
  llvm/include/llvm/IR/Module.h
  llvm/include/llvm/Transforms/Utils/BuildLibCalls.h
  llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h
  llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp
  llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
  llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
  llvm/lib/Transforms/Utils/BuildLibCalls.cpp
  llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
  llvm/test/Transforms/InferFunctionAttrs/annotate.ll
  llvm/test/Transforms/InstCombine/SystemZ/libcall-arg-exts.ll
  llvm/test/Transforms/InstCombine/double-float-shrink-1.ll
  llvm/test/Transforms/InstCombine/pr39177.ll
  llvm/test/Transforms/InstCombine/simplify-libcalls.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D123198.426015.patch
Type: text/x-patch
Size: 62707 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220429/42358a99/attachment-0001.bin>