This is an archive of the discontinued LLVM Phabricator instance.

Differential D109521

Remove obsolete msvc workaround from AddLLVM.cmake
ClosedPublic

Authored by stella.stamenova on Sep 9 2021, 9:27 AM.

Details

Reviewers
mehdi_amini
Commits
rG584a5d171eca: Remove obsolete msvc workaround from AddLLVM.cmake
Summary

The original change to add the workaround is from 10 years ago and a lot has happened with msvc and cmake and llvm's usage of cmake since and we no longer need the workaround for any scenarios that I am aware of. Build more is now correctly configured for multi-configuration generators such as Visual Studio.

The workaround is, however, causing issues with some of the recent mlir tests as because of the workaround we cannot correctly determine whether assertions are enabled (see https://reviews.llvm.org/D105961).

The original change is:

commit b46fdac4609df2613177813d43a124e3d9a8a306
Author: Andrew Trick <atrick@apple.com>
Date:   Tue Jun 28 16:32:01 2011

    cmake: Our MSVC build does not support config-time build mode.
    
    llvm-svn: 134008

Diff Detail

Event Timeline

stella.stamenova created this revision.Sep 9 2021, 9:27 AM
Herald added subscribers: Chia-hungDuan, rriddle, mgorny. · View Herald TranscriptSep 9 2021, 9:27 AM
stella.stamenova requested review of this revision.Sep 9 2021, 9:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 9 2021, 9:27 AM
Herald added subscribers: llvm-commits, stephenneuendorffer. · View Herald Transcript
mehdi_amini accepted this revision.Sep 9 2021, 10:47 AM
This revision is now accepted and ready to land.Sep 9 2021, 10:47 AM
This revision was landed with ongoing or failed builds.Sep 9 2021, 10:50 AM
Closed by commit rG584a5d171eca: Remove obsolete msvc workaround from AddLLVM.cmake (authored by stella.stamenova). · Explain Why
This revision was automatically updated to reflect the committed changes.
stella.stamenova added a commit: rG584a5d171eca: Remove obsolete msvc workaround from AddLLVM.cmake.
Harbormaster completed remote builds in B123243: Diff 371623.Sep 9 2021, 11:45 AM

Revision Contents

PathSize
llvm/
cmake/
modules/
2 lines

Diff 371656

llvm/cmake/modules/AddLLVM.cmake

Show First 20 Lines • Show All 1,632 Lines • ▼ Show 20 Linesfunction(configure_lit_site_cfg site_in site_out)
# FIXME: "ENABLE_SHARED" doesn't make sense, since it is used just for # FIXME: "ENABLE_SHARED" doesn't make sense, since it is used just for
# plugins. We may rename it. # plugins. We may rename it.
if(LLVM_ENABLE_PLUGINS) if(LLVM_ENABLE_PLUGINS)
set(ENABLE_SHARED "1") set(ENABLE_SHARED "1")
else() else()
set(ENABLE_SHARED "0") set(ENABLE_SHARED "0")
endif() endif()
if(LLVM_ENABLE_ASSERTIONS AND NOT MSVC_IDE) if(LLVM_ENABLE_ASSERTIONS)
set(ENABLE_ASSERTIONS "1") set(ENABLE_ASSERTIONS "1")
else() else()
set(ENABLE_ASSERTIONS "0") set(ENABLE_ASSERTIONS "0")
endif() endif()
set(HOST_OS ${CMAKE_SYSTEM_NAME}) set(HOST_OS ${CMAKE_SYSTEM_NAME})
set(HOST_ARCH ${CMAKE_SYSTEM_PROCESSOR}) set(HOST_ARCH ${CMAKE_SYSTEM_PROCESSOR})
▲ Show 20 Lines • Show All 564 Lines • Show Last 20 Lines