This is an archive of the discontinued LLVM Phabricator instance.

MemoryBuiltins: only claim an allocator family on builtin functions
ClosedPublic

Authored by durin42 on Apr 4 2022, 10:45 AM.

Details

Summary

This lines up with other parts of the codebase that only use special
knowledge about allocator functions if they're builtins.

Diff Detail

Event Timeline

durin42 created this revision.Apr 4 2022, 10:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 4 2022, 10:45 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
durin42 requested review of this revision.Apr 4 2022, 10:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 4 2022, 10:45 AM
nikic added a subscriber: nikic.Apr 6 2022, 2:21 AM

Can this be tested?

durin42 updated this revision to Diff 420850.Apr 6 2022, 7:41 AM
durin42 edited the summary of this revision. (Show Details)
durin42 updated this revision to Diff 420911.Apr 6 2022, 9:10 AM

Can this be tested?

I don't really think so. It shows up much later in the work when we start futzing with clang to move operator::new over to attributes from the hard-coded list, but it's really mostly transitional IMO because once everything is wired up in clang I think we'll be able to jettison the nobuiltin hack on those functions anyway.

Basically this is for symmetry with the other allocation function checks in MemoryBuiltins, wherein if a function is marked nobuiltin and the call isn't overriding that with builtin, we pretend we don't know anything. That's a hack so we don't elide non-elidable operator::new calls (we can elide generated ones, but not explicit ones, or something like that - it's a little past my ability to parse the C++ standard bits there), and I think the end result of the attributes work I'm doing will be to dump that hack.

Does that make you feel enough better? Want any of this in the commit message?

durin42 updated this revision to Diff 421204.Apr 7 2022, 7:36 AM
nikic accepted this revision.Apr 7 2022, 7:59 AM

LGTM

This revision is now accepted and ready to land.Apr 7 2022, 7:59 AM
This revision was landed with ongoing or failed builds.Apr 7 2022, 9:39 AM
This revision was automatically updated to reflect the committed changes.