This is an archive of the discontinued LLVM Phabricator instance.

[CFLAA] Move a common function to the utils header to reduce duplication
ClosedPublic

Authored by davide on Jun 26 2017, 5:52 PM.

Details

Summary

In the same vein as my last change. I'm not convinced about using inline in the header to force the function to live in a COMDAT, so, suggestions welcome.

Diff Detail

Repository
rL LLVM

Event Timeline

davide created this revision.Jun 26 2017, 5:52 PM

LGTM. Thanks!

include/llvm/Analysis/CFLAliasAnalysisUtils.h
23 ↗(On Diff #104061)

Now that I look at it, this should probably live in the llvm::cflaa namespace, rather than the global namespace (unless you're planning on making this more general?) As an added bonus, that may let us get rid of the using namespace llvm above?

I'm indifferent about whether that's included in this patch, or if it's done in a follow-up.

46 ↗(On Diff #104061)

I'm not convinced about using inline in the header to force the function to live in a COMDAT, so, suggestions welcome

Honestly, given how small this function will end up being and that it should only ever be used in 2 TUs, I'd be content if we just made it static inline.

I'm happy with either approach ¯\_(ツ)_/¯.

56 ↗(On Diff #104061)

If we expand these namespaces to include FunctionHandle, please add // namespace cflaa and // namespace llvm to their respective close braces.

This revision is now accepted and ready to land.Jun 26 2017, 6:51 PM
This revision was automatically updated to reflect the committed changes.

r306354/r306355, thanks