This is an archive of the discontinued LLVM Phabricator instance.

Fix LLVM tool --version build mode printing for MSVC
ClosedPublic

Authored by rnk on Dec 22 2019, 1:28 PM.

Details

Summary

LLVM tools such as llc print "DEBUG build" or "Optimized build" when
passed --version. Before this change, this was implemented by checking
for the OPTIMIZE GCC macro. MSVC does not define this macro. For
MSVC, control this behavior with _DEBUG instead. It doesn't have
precisely the same meaning, but in most configurations, it will do the
right thing.

Fixes PR17752

Diff Detail

Event Timeline

rnk created this revision.Dec 22 2019, 1:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 22 2019, 1:28 PM
Herald added a subscriber: hiraditya. · View Herald Transcript

Unit tests: pass. 61086 tests passed, 0 failed and 728 were skipped.

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

MaskRay added inline comments.
llvm/lib/Support/CommandLine.cpp
2443

LLVM_IS_OPTIMIZED_BUILD looks like more appropriate?
I thought the difference is between "optimized/non-optimized builds", not between "debug/non-debug builds".

rnk marked an inline comment as done.Dec 22 2019, 2:59 PM
rnk added inline comments.
llvm/lib/Support/CommandLine.cpp
2443

I figured this whole feature exists to warn users when they are running tools built in debug mode, so I felt this was the better direction. But maybe I am biased, I develop with optimizations enabled. =P

xbolva00 added inline comments.Dec 22 2019, 3:21 PM
llvm/lib/Support/CommandLine.cpp
2439

Move to compiler.h?

MaskRay accepted this revision.Dec 22 2019, 3:28 PM
This revision is now accepted and ready to land.Dec 22 2019, 3:28 PM

I agree the LLVM_IS_DEBUG_BUILD defs should probably be in Compiler.h

rnk marked an inline comment as done.Dec 23 2019, 9:59 AM
rnk added inline comments.
llvm/lib/Support/CommandLine.cpp
2439

This is the only existing use of __OPTIMIZE__ in LLVM or clang, so I would prefer to keep it local until there is another use. I hope there will never be one, it's generally not a good idea to ifdef on whether optimizations are enabled.

This comment was removed by xbolva00.
llvm/lib/Support/CommandLine.cpp
2439
rnk marked an inline comment as done.Dec 23 2019, 10:06 AM
rnk added inline comments.
llvm/lib/Support/CommandLine.cpp
2439

I did, but that seems like a better candidate for #ifdef EXPENSIVE_CHECKS.

This revision was automatically updated to reflect the committed changes.