Skip to content

Commit

Permalink
Deprecate unified build in favor of unity build. (#3491)
Browse files Browse the repository at this point in the history
* Remove the unified build.

Fix build.

Missing build_unified.

Clean up cmakelists.

Fix build flags.

Need to include GENERATED property.

Wrap clashing variable names in appropriate namespace.

Cpplint.

Simplify CI. Aggressive batch size.

Fix template.

cpplint.

Build fixes.

Fixup.

* Legacy options for unified compilation.

* Warning.

* Versioning.

* Install CMake 3.16.3 for Ubuntu 18.04.

* Cosmetic edits.

* Do not use generic namespaces when boost is present.

* Do not ignore Gauntlet build issues.

* Rename p4runtime functions because of conflict gmock macro.
  • Loading branch information
fruffy authored Feb 23, 2023
1 parent 3e9bf6b commit ca1f347
Show file tree
Hide file tree
Showing 47 changed files with 214 additions and 180 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/ci-p4tools.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
env:
CTEST_PARALLEL_LEVEL: 4
IMAGE_TYPE: test
ENABLE_UNIFIED_COMPILATION: ON
CMAKE_UNITY_BUILD: ON
ENABLE_TEST_TOOLS: ON
steps:
- uses: actions/checkout@v3
Expand All @@ -30,7 +30,7 @@ jobs:
- name: ccache
uses: hendrikmuhs/ccache-action@v1
with:
key: test-tools-${{ matrix.unified }}-${{ runner.os }}
key: test-tools-${{ runner.os }}
max-size: 1000M

- name: Build (Ubuntu 20.04)
Expand Down
3 changes: 1 addition & 2 deletions .github/workflows/ci-ptf-kernels-weekly.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ jobs:
kernel_version: 5.15.0-48
os: ubuntu-20.04
env:
UNIFIED: ON
# Used by virt-builder and virt-install, for valid values see `virt-builder --list`.
OS_TYPE: ${{ matrix.os }}
# List of kernels to use in a single job
Expand Down Expand Up @@ -78,7 +77,7 @@ jobs:

- name: Build (Linux)
run: |
docker build --network host -t p4c --build-arg MAKEFLAGS=-j8 --build-arg IMAGE_TYPE=test --build-arg ENABLE_UNIFIED_COMPILATION=$UNIFIED --build-arg INSTALL_PTF_EBPF_DEPENDENCIES=ON --build-arg KERNEL_VERSIONS="$KERNEL_VERSIONS" .
docker build --network host -t p4c --build-arg MAKEFLAGS=-j8 --build-arg IMAGE_TYPE=test --build-arg CMAKE_UNITY_BUILD=ON --build-arg INSTALL_PTF_EBPF_DEPENDENCIES=ON --build-arg KERNEL_VERSIONS="$KERNEL_VERSIONS" .
./tools/export_ccache.sh
- name: Install VM
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci-ptf.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
env:
CTEST_PARALLEL_LEVEL: 4
IMAGE_TYPE: test
ENABLE_UNIFIED_COMPILATION: ON
CMAKE_UNITY_BUILD: ON
MAKEFLAGS: -j8
INSTALL_PTF_EBPF_DEPENDENCIES: ON
steps:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci-static-build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
runs-on: ubuntu-20.04
env:
IMAGE_TYPE: test
ENABLE_UNIFIED_COMPILATION: ON
CMAKE_UNITY_BUILD: ON
BUILD_STATIC_RELEASE: ON
steps:
- uses: actions/checkout@v3
Expand Down
22 changes: 9 additions & 13 deletions .github/workflows/ci-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ jobs:
env:
CTEST_PARALLEL_LEVEL: 4
IMAGE_TYPE: test
ENABLE_UNIFIED_COMPILATION: ON
steps:
- uses: actions/checkout@v3
with:
Expand All @@ -44,16 +43,16 @@ jobs:

# Build with gcc and test p4c on Ubuntu 20.04.
test-ubuntu20:
name: test-ubuntu20 (Unified ${{ matrix.unified }})
name: test-ubuntu20 (Unity ${{ matrix.unity }})
strategy:
fail-fast: false
matrix:
unified: [ON, OFF]
unity: [ON, OFF]
runs-on: ubuntu-20.04
env:
CTEST_PARALLEL_LEVEL: 4
IMAGE_TYPE: test
ENABLE_UNIFIED_COMPILATION: ${{ matrix.unified }}
CMAKE_UNITY_BUILD: ${{ matrix.unity }}
steps:
- uses: actions/checkout@v3
with:
Expand All @@ -63,7 +62,7 @@ jobs:
- name: ccache
uses: hendrikmuhs/ccache-action@v1
with:
key: test-${{ matrix.unified }}-${{ runner.os }}-gcc
key: test-${{ matrix.unity }}-${{ runner.os }}-gcc
max-size: 1000M

- name: Build (Ubuntu 20.04, GCC)
Expand All @@ -74,7 +73,7 @@ jobs:
# Need to use sudo for the eBPF kernel tests.
run: sudo -E ctest --output-on-failure --schedule-random
working-directory: ./build
if: matrix.unified == 'ON'
if: matrix.unity == 'ON'

# Build with clang and test p4c on Ubuntu 20.04.
test-ubuntu20-clang-sanitizers:
Expand All @@ -84,7 +83,6 @@ jobs:
env:
CTEST_PARALLEL_LEVEL: 2
IMAGE_TYPE: test
ENABLE_UNIFIED_COMPILATION: ON
COMPILE_WITH_CLANG: ON
BUILD_AUTO_VAR_INIT_PATTERN: ON
ENABLE_SANITIZERS: ON
Expand Down Expand Up @@ -117,7 +115,6 @@ jobs:
runs-on: ubuntu-latest
env:
CTEST_PARALLEL_LEVEL: 4
ENABLE_UNIFIED_COMPILATION: ON
steps:
- uses: actions/checkout@v3
with:
Expand All @@ -132,7 +129,7 @@ jobs:

- name: Build (Ubuntu 18.04, GCC)
run: |
docker build -t p4c --build-arg BASE_IMAGE=ubuntu:18.04 --build-arg IMAGE_TYPE=test --build-arg ENABLE_UNIFIED_COMPILATION=ON .
docker build -t p4c --build-arg BASE_IMAGE=ubuntu:18.04 --build-arg IMAGE_TYPE=test .
./tools/export_ccache.sh
# run with sudo (...) --privileged
Expand Down Expand Up @@ -172,9 +169,8 @@ jobs:

- name: Build p4c (Fedora Linux)
run: |
./bootstrap.sh -DCMAKE_BUILD_TYPE=RELEASE -DENABLE_UNIFIED_COMPILATION=ON
make -j2 -C build
./bootstrap.sh -DCMAKE_BUILD_TYPE=RELEASE -DCMAKE_UNITY_BUILD=ON
make -j2 -C build
- name: Run p4c tests (Fedora Linux)
# Need to use sudo for the eBPF kernel tests.
Expand Down Expand Up @@ -213,7 +209,7 @@ jobs:
run: |
PROTOBUF=`brew --prefix --installed protobuf`
./bootstrap.sh -DENABLE_GC=ON -DCMAKE_BUILD_TYPE=RELEASE \
-DENABLE_UNIFIED_COMPILATION=ON \
-DCMAKE_UNITY_BUILD=ON \
-DProtobuf_INCLUDE_DIR=$PROTOBUF/include \
-DProtobuf_LIBRARY=$PROTOBUF/lib/libprotobuf.dylib \
-DENABLE_PROTOBUF_STATIC=OFF
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/ci-validation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ jobs:
env:
CTEST_PARALLEL_LEVEL: 4
IMAGE_TYPE: test
ENABLE_UNIFIED_COMPILATION: ON
VALIDATION: ON
runs-on: ubuntu-20.04
steps:
Expand All @@ -38,5 +37,6 @@ jobs:
tools/ci-build.sh
- name: Validate
run: ctest -R toz3-validate-p4c --output-on-failure --schedule-random
run: |
ctest -R toz3-validate-p4c --output-on-failure --schedule-random
working-directory: ./build
21 changes: 18 additions & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# This is the CMake version supplied by Ubuntu 18.04.
cmake_minimum_required (VERSION 3.10.2 FATAL_ERROR)
# This is the CMake version supplied by Ubuntu 20.04.
cmake_minimum_required (VERSION 3.16.3 FATAL_ERROR)

find_program(CCACHE_PROGRAM ccache)
if(CCACHE_PROGRAM)
Expand Down Expand Up @@ -47,6 +47,8 @@ OPTION (ENABLE_SANITIZERS "Enable sanitizers" OFF)
OPTION (BUILD_STATIC_RELEASE "Build a statically linked release binary" OFF)
OPTION (BUILD_AUTO_VAR_INIT_PATTERN "Initialize variables with pattern during build" OFF)
OPTION (ENABLE_IWYU "Enable checking includes with IWYU" OFF)
# Support a legacy option. TODO: Remove?
OPTION (ENABLE_UNIFIED_COMPILATION "Enable CMAKE_UNITY_BUILD" OFF)

set (P4C_DRIVER_NAME "p4c" CACHE STRING "Customize the name of the driver script")

Expand Down Expand Up @@ -83,13 +85,26 @@ else()
set (P4C_VERSION "${P4C_SEM_VERSION_STRING} (SHA: ${P4C_GIT_SHA} BUILD: ${CMAKE_BUILD_TYPE})")
endif()
include(P4CUtils)
include(UnifiedBuild)

# # search in /usr/local first
# set (CMAKE_FIND_ROOT_PATH "/usr/local/bin" "${CMAKE_FIND_ROOT_PATH}")
set (P4C_CXX_FLAGS "")
set (P4C_LIB_DEPS)


# Support the legacy unified build option.
if(ENABLE_UNIFIED_COMPILATION)
message(
WARNING
"Using deprecated option \"ENABLE_UNIFIED_COMPILATION\". Please use \"CMAKE_UNITY_BUILD\" instead."
)
set(CMAKE_UNITY_BUILD ON)
endif()

# If unity builds are enabled, choose an aggressive batch size.
if (CMAKE_UNITY_BUILD)
set(CMAKE_UNITY_BUILD_BATCH_SIZE 16 CACHE UNINITIALIZED "Set the unity build batch size.")
endif ()
# set the required options for a static release build
if (BUILD_STATIC_RELEASE)
message(STATUS "Building static release binaries")
Expand Down
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ ARG IN_DOCKER=TRUE
# testing; in this case, the source code and build-only dependencies will not be
# removed from the image.
ARG IMAGE_TYPE=build
# Whether to do a unified build.
ARG ENABLE_UNIFIED_COMPILATION=ON
# Whether to do a unity build.
ARG CMAKE_UNITY_BUILD=ON
# Whether to enable translation validation
ARG VALIDATION=OFF
# This creates a release build that includes link time optimization and links
Expand Down
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ use them, but YMMV.

- `git` for version control

- CMake 3.10.2 or higher
- CMake 3.16.3 or higher

- Boehm-Weiser garbage-collector C++ library

Expand Down Expand Up @@ -283,6 +283,9 @@ Please note that while all Protobuf versions newer than 3.0 should work for
`p4c` itself, you may run into trouble with some extensions and other p4lang
projects unless you install version 3.18.1.

### CMake
p4c requires a CMake version of at least 3.16.3 or higher. On older systems, a newer version of CMake can be installed using `pip3 install --user cmake==3.16.3`. We have a CI test on Ubuntu 18.04 that uses this option, but there is no guarantee that this will lead to a successful build.

## Fedora dependencies

```bash
Expand Down
3 changes: 0 additions & 3 deletions backends/bmv2/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -82,17 +82,14 @@ set (BMV2_BACKEND_COMMON_HDRS

set (IR_DEF_FILES ${IR_DEF_FILES} ${CMAKE_CURRENT_SOURCE_DIR}/bmv2.def PARENT_SCOPE)

build_unified(BMV2_BACKEND_COMMON_SRCS)
add_library(bmv2backend ${P4C_STATIC_BUILD} ${BMV2_BACKEND_COMMON_SRCS})
add_dependencies(bmv2backend genIR frontend)

build_unified(BMV2_SIMPLE_SWITCH_SRCS)
add_executable(p4c-bm2-ss ${BMV2_SIMPLE_SWITCH_SRCS})
target_link_libraries (p4c-bm2-ss bmv2backend ${P4C_LIBRARIES} ${P4C_LIB_DEPS})

install(TARGETS p4c-bm2-ss RUNTIME DESTINATION ${P4C_RUNTIME_OUTPUT_DIRECTORY})

build_unified(BMV2_PSA_SWITCH_SRCS)
add_executable(p4c-bm2-psa ${BMV2_PSA_SWITCH_SRCS})
target_link_libraries (p4c-bm2-psa bmv2backend ${P4C_LIBRARIES} ${P4C_LIB_DEPS})
install(TARGETS p4c-bm2-psa RUNTIME DESTINATION ${P4C_RUNTIME_OUTPUT_DIRECTORY})
Expand Down
19 changes: 10 additions & 9 deletions backends/dpdk/control-plane/bfruntime_arch_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class SymbolTypeDPDK final : public SymbolType {
public:
SymbolTypeDPDK() = delete;

static P4RuntimeSymbolType ACTION_SELECTOR() {
static P4RuntimeSymbolType P4RT_ACTION_SELECTOR() {
return P4RuntimeSymbolType::make(dpdk::P4Ids::ACTION_SELECTOR);
}
};
Expand All @@ -76,7 +76,7 @@ struct ActionSelector {
static constexpr int64_t defaultMaxGroupSize = 120;

p4rt_id_t getId(const P4RuntimeSymbolTableIface &symbols) const {
return symbols.getId(SymbolTypeDPDK::ACTION_SELECTOR(), name + "_sel");
return symbols.getId(SymbolTypeDPDK::P4RT_ACTION_SELECTOR(), name + "_sel");
}
};

Expand Down Expand Up @@ -184,16 +184,17 @@ class BFRuntimeArchHandler : public P4RuntimeArchHandlerCommon<arch> {
auto tablesIt = this->actionProfilesRefs.find(actionSelector.name);
if (tablesIt != this->actionProfilesRefs.end()) {
for (const auto &table : tablesIt->second) {
profile.add_table_ids(symbols.getId(P4RuntimeSymbolType::TABLE(), table));
selector.add_table_ids(symbols.getId(P4RuntimeSymbolType::TABLE(), table));
profile.add_table_ids(symbols.getId(P4RuntimeSymbolType::P4RT_TABLE(), table));
selector.add_table_ids(symbols.getId(P4RuntimeSymbolType::P4RT_TABLE(), table));
}
}
// We use the ActionSelector name for the action profile, and add a "_sel" suffix for
// the action selector.
cstring profileName = actionSelector.name;
selector.set_action_profile_id(symbols.getId(SymbolType::ACTION_PROFILE(), profileName));
selector.set_action_profile_id(
symbols.getId(SymbolType::P4RT_ACTION_PROFILE(), profileName));
cstring selectorName = profileName + "_sel";
addP4InfoExternInstance(symbols, SymbolTypeDPDK::ACTION_SELECTOR(), "ActionSelector",
addP4InfoExternInstance(symbols, SymbolTypeDPDK::P4RT_ACTION_SELECTOR(), "ActionSelector",
selectorName, actionSelector.annotations, selector, p4Info,
pipeName);
}
Expand All @@ -205,12 +206,12 @@ class BFRuntimeArchHandler : public P4RuntimeArchHandlerCommon<arch> {
auto decl = externBlock->node->to<IR::IDeclaration>();
if (decl == nullptr) return;
if (externBlock->type->name == "Digest") {
symbols->add(SymbolType::DIGEST(), decl);
symbols->add(SymbolType::P4RT_DIGEST(), decl);
} else if (externBlock->type->name == ActionSelectorTraits<arch>::typeName()) {
auto selName = decl->controlPlaneName() + "_sel";
auto profName = decl->controlPlaneName();
symbols->add(SymbolTypeDPDK::ACTION_SELECTOR(), selName);
symbols->add(SymbolType::ACTION_PROFILE(), profName);
symbols->add(SymbolTypeDPDK::P4RT_ACTION_SELECTOR(), selName);
symbols->add(SymbolType::P4RT_ACTION_PROFILE(), profName);
}
}

Expand Down
1 change: 0 additions & 1 deletion backends/ebpf/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ set (P4C_EBPF_HDRS

set (P4C_EBPF_DIST_HEADERS p4include/ebpf_model.p4)

build_unified(P4C_EBPF_SRCS)
add_executable(p4c-ebpf ${P4C_EBPF_SRCS})
target_link_libraries (p4c-ebpf ${P4C_LIBRARIES} ${P4C_LIB_DEPS})
add_dependencies(p4c-ebpf genIR frontend)
Expand Down
2 changes: 1 addition & 1 deletion backends/graphs/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ set (GRAPHS_HDRS
graph_visitor.h
)

build_unified(GRAPHS_SRCS ALL)

add_executable(p4c-graphs ${GRAPHS_SRCS} ${EXTENSION_P4_14_CONV_SOURCES})
target_link_libraries (p4c-graphs ${P4C_LIBRARIES} ${P4C_LIB_DEPS})
add_dependencies(p4c-graphs genIR frontend)
Expand Down
2 changes: 1 addition & 1 deletion backends/p4test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ set (P4TEST_HDRS
midend.h
)

build_unified(P4TEST_SRCS ALL)

add_executable(p4test ${P4TEST_SRCS} ${EXTENSION_P4_14_CONV_SOURCES})
target_link_libraries (p4test ${P4C_LIBRARIES} ${P4C_LIB_DEPS})
add_dependencies(p4test genIR frontend)
Expand Down
4 changes: 2 additions & 2 deletions backends/p4tools/modules/testgen/register.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ namespace P4Tools {

namespace P4Testgen {

void registerCompilerTargets() {
inline void registerCompilerTargets() {
@compiler_targets_var@}

void registerTestgenTargets() {
inline void registerTestgenTargets() {
@testgen_targets_var@}

} // namespace P4Tools
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ boost::optional<p4rt_id_t> Protobuf::getIdAnnotation(const IR::IAnnotated *node)
return static_cast<p4rt_id_t>(idConstant->value);
}

boost::optional<p4rt_id_t> Protobuf::externalId(const P4RuntimeSymbolType &type,
const IR::IDeclaration *declaration) {
boost::optional<p4rt_id_t> Protobuf::externalId(
const P4::ControlPlaneAPI::P4RuntimeSymbolType &type, const IR::IDeclaration *declaration) {
CHECK_NULL(declaration);
if (!declaration->is<IR::IAnnotated>()) {
return boost::none; // Assign an id later; see below.
Expand Down Expand Up @@ -129,7 +129,7 @@ inja::json Protobuf::getControlPlane(const TestSpec *testSpec) {
const auto *const tblConfig = testObject.second->checkedTo<TableConfig>();
const auto *table = tblConfig->getTable();

auto p4RuntimeId = externalId(SymbolType::TABLE(), table);
auto p4RuntimeId = externalId(SymbolType::P4RT_TABLE(), table);
BUG_CHECK(p4RuntimeId, "Id not present for table %1%. Can not generate test.", table);
tblJson["id"] = *p4RuntimeId;

Expand All @@ -142,7 +142,7 @@ inja::json Protobuf::getControlPlane(const TestSpec *testSpec) {
const auto *actionDecl = actionCall->getAction();
const auto *actionArgs = actionCall->getArgs();
rule["action_name"] = actionCall->getActionName().c_str();
auto p4RuntimeId = externalId(SymbolType::ACTION(), actionDecl);
auto p4RuntimeId = externalId(SymbolType::P4RT_ACTION(), actionDecl);
BUG_CHECK(p4RuntimeId, "Id not present for action %1%. Can not generate test.",
actionDecl);
rule["action_id"] = *p4RuntimeId;
Expand Down Expand Up @@ -277,7 +277,7 @@ inja::json Protobuf::getVerify(const TestSpec *testSpec) {
return verifyData;
}

static std::string getTestCase() {
std::string Protobuf::getTestCaseTemplate() {
static std::string TEST_CASE(
R"""(# A P4TestGen-generated test case for {{test_name}}.p4
metadata: "p4testgen seed: {{ default(seed, "none") }}"
Expand Down Expand Up @@ -429,7 +429,7 @@ void Protobuf::outputTest(const TestSpec *testSpec, cstring selectedBranches, si
auto incrementedTestName = testName + "_" + std::to_string(testIdx);

protobufFile = std::ofstream(incrementedTestName + ".proto");
std::string testCase = getTestCase();
std::string testCase = getTestCaseTemplate();
emitTestcase(testSpec, selectedBranches, testIdx, testCase, currentCoverage);
}

Expand Down
Loading

0 comments on commit ca1f347

Please sign in to comment.