This is an archive of the discontinued LLVM Phabricator instance.

Remove compile time PreserveName in favor of a runtime cc1 -discard-value-names option
ClosedPublic

Authored by mehdi_amini on Mar 12 2016, 5:58 PM.

Details

Summary

This flag is enabled by default in the driver when NDEBUG is set. It
is forwarded on the LLVMContext to discard all value names (but
GlobalValue) for performance purpose.

This an improved version of D18024

Diff Detail

Event Timeline

mehdi_amini retitled this revision from to Remove compile time PreserveName in favor of a runtime cc1 -discard-value-names option.
mehdi_amini updated this object.
mehdi_amini added reviewers: echristo, chandlerc.
mehdi_amini added a subscriber: cfe-commits.
chandlerc edited edge metadata.Mar 12 2016, 8:04 PM

Change these tests to use %clang_cc1 rather than add all the messy regex to handle arguments losing names?

lib/Driver/Tools.cpp
3715

s/Disable/Discard/

mehdi_amini edited edge metadata.

Use cc1 in the few tests that were relying on value names. Also add test/CodeGenCXX/discard-name-values.cpp to test the new cc1 flag.

s/clang -cc1/%clang_cc1/

chandlerc accepted this revision.Mar 13 2016, 6:07 AM
chandlerc edited edge metadata.

Looks good with the two test nits below fixed.

test/CodeGen/mips-zero-sized-struct.c
8–10

No need for this part of the change now?

test/CodeGenCXX/stack-reuse.cpp
1

Another place to use %clang_cc1 instead of '-cc1'.

This revision is now accepted and ready to land.Mar 13 2016, 6:07 AM

Interesting question going forwards: We usually try to make test cases
+/-Asserts agnostic (not using named values), now that we have a flag for
it, should we not bother with that & just add the command line argument to
enable names when names make testing easier? Or do we think that'll make
tests more brittle/depend on unnecessary implementation details?