This is an archive of the discontinued LLVM Phabricator instance.

BuildLibCalls: add alloc-family attribute to many allocator functions
ClosedPublic

Authored by durin42 on Apr 4 2022, 3:17 PM.

Diff Detail

Event Timeline

durin42 created this revision.Apr 4 2022, 3:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 4 2022, 3:17 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
durin42 requested review of this revision.Apr 4 2022, 3:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 4 2022, 3:17 PM
durin42 updated this revision to Diff 420855.Apr 6 2022, 7:41 AM
durin42 edited the summary of this revision. (Show Details)
durin42 updated this revision to Diff 420916.Apr 6 2022, 9:10 AM
durin42 updated this revision to Diff 421208.Apr 7 2022, 7:36 AM
durin42 updated this revision to Diff 422243.Apr 12 2022, 8:33 AM
durin42 updated this revision to Diff 424969.Apr 25 2022, 10:55 AM
durin42 updated this revision to Diff 425068.Apr 25 2022, 5:15 PM
durin42 updated this revision to Diff 425362.Apr 26 2022, 5:34 PM
nikic added a subscriber: nikic.Apr 27 2022, 12:31 AM

This needs a LangRef patch to specify alloc-family first.

durin42 updated this revision to Diff 425494.Apr 27 2022, 6:05 AM

This needs a LangRef patch to specify alloc-family first.

LangRef updated, thanks!

durin42 updated this revision to Diff 425837.Apr 28 2022, 10:34 AM
nikic added a reviewer: nikic.Apr 29 2022, 1:08 AM
nikic added inline comments.
llvm/docs/LangRef.rst
1570 ↗(On Diff #425837)

Wrap ::operator::new etc in double backticks?

1571 ↗(On Diff #425837)

This doesn't specify what the semantics of the attribute are. I'd expect something along the lines of "only malloc/free pairs of the same allocator family can be optimized".

durin42 updated this revision to Diff 426085.Apr 29 2022, 9:29 AM
durin42 marked 2 inline comments as done.Apr 29 2022, 9:35 AM
durin42 updated this revision to Diff 426405.May 2 2022, 7:04 AM
nikic accepted this revision.May 2 2022, 7:23 AM

LG

This revision is now accepted and ready to land.May 2 2022, 7:23 AM
nikic added inline comments.May 2 2022, 12:36 PM
llvm/lib/Transforms/Utils/BuildLibCalls.cpp
458

I just looked at this again, and while this does actually end up doing the right thing, I think it's very subtle. It would be better to rewrite this like so (without the separate fallthrough):

Changed |= setAllocFamility(F, TheLibFunc == LibFunc_vec_malloc ? "vec_malloc" : "malloc");

It also looks like we currently don't have tests for the vec_malloc case.