This is an archive of the discontinued LLVM Phabricator instance.

[SimplifyLibCalls] Fold more fortified builtin functions into their non-fortified variants when possible
ClosedPublic

Authored by erik.pilkington on May 23 2019, 5:11 PM.

Details

Summary

When the object size argument is -1, no checking can be done, so calling the _chk variant is unnecessary. We already did this for a bunch of these functions.

rdar://50797197

Thanks for taking a look!

Diff Detail

Repository
rL LLVM

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMay 23 2019, 5:11 PM
jdoerfert accepted this revision.May 30 2019, 6:12 PM
jdoerfert added a subscriber: jdoerfert.

I added some minor comments but other than that I think it is fine.
I would, however, prefer it if you could split the patch up when you commit it so that the generalization of the builder is separated from the new fortified libcalls and their optimization.

llvm/include/llvm/Transforms/Utils/BuildLibCalls.h
109 ↗(On Diff #201100)

Any reason you didn't add comments here?

llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h
59 ↗(On Diff #201100)

Newline removed.

74 ↗(On Diff #201100)

When you add parameter descriptions, could you add it for all five parameters?

llvm/lib/Transforms/Utils/BuildLibCalls.cpp
792 ↗(On Diff #201100)

This can be commited separately.

This revision is now accepted and ready to land.May 30 2019, 6:12 PM
This revision was automatically updated to reflect the committed changes.
erik.pilkington marked 7 inline comments as done.May 31 2019, 3:39 PM
erik.pilkington added inline comments.
llvm/include/llvm/Transforms/Utils/BuildLibCalls.h
109 ↗(On Diff #201100)

No reason, I added them in the commit.

llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h
74 ↗(On Diff #201100)

Sure, done.

llvm/lib/Transforms/Utils/BuildLibCalls.cpp
792 ↗(On Diff #201100)

Sure, done in r362271. Thanks for reviewing!