[Build] Use -msse2 option only for 32-bit Intel (#38024)

This might not be needed? (From https://github.com/grpc/grpc/issues/37976)

- Boringssl commit requiring -msse2: 56d3ad9d23 (diff-f628d148f94bbab9e22a1ad426ccd94311f1fb87bbe5cc533cb85aee18b07a20R248)
- gRPC change to add -msse2 https://github.com/grpc/grpc/pull/36089

---

Answer to the quesiton above is yes; The -msse2 option remains necessary for gRPC on i686 due to BoringSSL's requirements. However, the existing CMake condition for this option was too broad, potentially including ARM architectures where SSE2 isn't supported, leading to compilation errors. I've refined the condition to specifically target 32-bit x86 architectures.

Furthermore, to ensure accurate architecture detection within our dockerized tests, I've configured x86 tests to utilize the linux32 command. This ensures that uname -a correctly reports i686, allowing gRPC's CMake to identify the architecture and apply the -msse2 option as needed.

It's important to note that RBE overrides the default entrypoint, so RBE-based tests must explicitly invoke linux32 even if the Docker image already has it set.

Fixes https://github.com/grpc/grpc/issues/37976

Closes #38024

PiperOrigin-RevId: 693026079
pull/37136/head
Esun Kim 3 weeks ago committed by Copybara-Service
parent cb57a0496c
commit 574b19ec31
  1. 4
      CMakeLists.txt
  2. 4
      templates/CMakeLists.txt.template
  3. 6
      templates/tools/dockerfile/test/cxx_debian11_x86/Dockerfile.template
  4. 8
      tools/bazelify_tests/build_defs.bzl
  5. 2
      tools/bazelify_tests/dockerimage_current_versions.bzl
  6. 2
      tools/bazelify_tests/grpc_run_tests_harness_test.sh
  7. 1
      tools/bazelify_tests/test/portability_tests.bzl
  8. 2
      tools/dockerfile/test/cxx_debian11_x86.current_version
  9. 6
      tools/dockerfile/test/cxx_debian11_x86/Dockerfile

4
CMakeLists.txt generated

@ -268,7 +268,9 @@ set(gRPC_USE_PROTO_LITE OFF CACHE BOOL "Use the protobuf-lite library")
if(UNIX)
if(${CMAKE_SYSTEM_NAME} MATCHES "Linux")
set(_gRPC_PLATFORM_LINUX ON)
if(NOT CMAKE_CROSSCOMPILING AND CMAKE_SIZEOF_VOID_P EQUAL 4)
# Boring SSL needs -msse2 build option on 32-bit x86
# See https://github.com/google/boringssl/commit/56d3ad9d23bc130aa9404bfdd1957fe81b3ba498
if(CMAKE_SYSTEM_PROCESSOR MATCHES "i[3-6]86" AND CMAKE_SIZEOF_VOID_P EQUAL 4)
message("+++ Enabling SSE2 for ${CMAKE_SYSTEM_PROCESSOR}")
set(_gRPC_C_CXX_FLAGS "${_gRPC_C_CXX_FLAGS} -msse2")
endif()

@ -374,7 +374,9 @@
if(UNIX)
if(<%text>${CMAKE_SYSTEM_NAME}</%text> MATCHES "Linux")
set(_gRPC_PLATFORM_LINUX ON)
if(NOT CMAKE_CROSSCOMPILING AND CMAKE_SIZEOF_VOID_P EQUAL 4)
# Boring SSL needs -msse2 build option on 32-bit x86
# See https://github.com/google/boringssl/commit/56d3ad9d23bc130aa9404bfdd1957fe81b3ba498
if(CMAKE_SYSTEM_PROCESSOR MATCHES "i[3-6]86" AND CMAKE_SIZEOF_VOID_P EQUAL 4)
message("+++ Enabling SSE2 for <%text>${CMAKE_SYSTEM_PROCESSOR}</%text>")
set(_gRPC_C_CXX_FLAGS "<%text>${_gRPC_C_CXX_FLAGS}</%text> -msse2")
endif()

@ -24,5 +24,7 @@
<%include file="../../ccache.include"/>
<%include file="../../run_tests_addons.include"/>
# Define the default command.
CMD ["bash"]
# docker is running on a 64-bit machine, so we need to
# override "uname -m" to report i686 instead of x86_64, otherwise
# cmake will choose a wrong option to build.
ENTRYPOINT ["linux32"]

@ -103,7 +103,7 @@ def _dockerized_genrule(name, cmd, outs, srcs = [], tags = [], exec_compatible_w
**genrule_args
)
def grpc_run_tests_harness_test(name, args = [], data = [], size = "medium", timeout = None, tags = [], exec_compatible_with = [], flaky = None, docker_image_version = None, use_login_shell = None, prepare_script = None):
def grpc_run_tests_harness_test(name, args = [], data = [], size = "medium", timeout = None, tags = [], exec_compatible_with = [], flaky = None, docker_image_version = None, use_login_shell = None, prepare_script = None, arch = None):
"""Execute an run_tests.py-harness style test under bazel.
Args:
@ -119,6 +119,7 @@ def grpc_run_tests_harness_test(name, args = [], data = [], size = "medium", tim
docker_image_version: The docker .current_version file to use for docker containerization.
use_login_shell: If True, the run_tests.py command will run under a login shell.
prepare_script: Optional script that will be sourced before run_tests.py runs.
arch: Optional a command to change reported architecture (such as linux32 for i686) for the test.
"""
data = [
@ -142,6 +143,11 @@ def grpc_run_tests_harness_test(name, args = [], data = [], size = "medium", tim
data = data + [prepare_script]
env["GRPC_RUNTESTS_PREPARE_SCRIPT"] = "$(location " + prepare_script + ")"
# Bazel RBE overrides ENTRYPOINT of docker so 32bits test relying on uname returning i686
# should have arch = "linux32" even when the docker file has it
if arch:
env["SETARCH_CMD"] = arch
# Enable ccache by default. This is important for speeding up the C++ cmake build,
# which isn't very efficient and tends to recompile some source files multiple times.
# Even though only the local disk cache is enabled (local to the docker container,

@ -98,7 +98,7 @@ DOCKERIMAGE_CURRENT_VERSIONS = {
"tools/dockerfile/test/cxx_debian11_openssl102_x64.current_version": "docker://us-docker.pkg.dev/grpc-testing/testing-images-public/cxx_debian11_openssl102_x64@sha256:477ae0da7ff7faa9cf195c0d32472fec4cf8b7325505c63e00b5c794c9a4b1a7",
"tools/dockerfile/test/cxx_debian11_openssl111_x64.current_version": "docker://us-docker.pkg.dev/grpc-testing/testing-images-public/cxx_debian11_openssl111_x64@sha256:d383e66d4a089f9305768e3037faa2a887ff91565b0f3ddd96985dca94e9754f",
"tools/dockerfile/test/cxx_debian11_x64.current_version": "docker://us-docker.pkg.dev/grpc-testing/testing-images-public/cxx_debian11_x64@sha256:9f9285da21c1053ac715027e0cee66c20c70ebf016053328a4cee61ffd37e59b",
"tools/dockerfile/test/cxx_debian11_x86.current_version": "docker://us-docker.pkg.dev/grpc-testing/testing-images-public/cxx_debian11_x86@sha256:3f505ad99e52a4b3337fedb413e883bc8e5c1d9c089299c34002b89e01254d3b",
"tools/dockerfile/test/cxx_debian11_x86.current_version": "docker://us-docker.pkg.dev/grpc-testing/testing-images-public/cxx_debian11_x86@sha256:43d6e6d97740a8f989024939b5507ed45071d00171eb68e1fbf101a9352860b1",
"tools/dockerfile/test/cxx_debian12_openssl309_x64.current_version": "docker://us-docker.pkg.dev/grpc-testing/testing-images-public/cxx_debian12_openssl309_x64@sha256:f75bb715c4f9464526f9affb410f7965a0b8894516d7d98cd89a4e165ae065b7",
"tools/dockerfile/test/cxx_gcc_14_x64.current_version": "docker://us-docker.pkg.dev/grpc-testing/testing-images-public/cxx_gcc_14_x64@sha256:54685fc729952b689318057a9769edc92247a40d607d01c3517d2644d361cc73",
"tools/dockerfile/test/cxx_gcc_7_x64.current_version": "docker://us-docker.pkg.dev/grpc-testing/testing-images-public/cxx_gcc_7_x64@sha256:e1925d7f08a7f167c6aab2f8284a3f13b7f3830ba38876e6de1dee2ffd3c7d2f",

@ -40,7 +40,7 @@ then
source "../${GRPC_RUNTESTS_PREPARE_SCRIPT}"
fi
python3 tools/run_tests/run_tests.py -t -j "$(nproc)" -x "${REPORT_XML_FILE}" --report_suite_name "${REPORT_SUITE_NAME}" "$@" || FAILED="true"
${SETARCH_CMD} python3 tools/run_tests/run_tests.py -t -j "$(nproc)" -x "${REPORT_XML_FILE}" --report_suite_name "${REPORT_SUITE_NAME}" "$@" || FAILED="true"
if [ -x "$(command -v ccache)" ]
then

@ -36,6 +36,7 @@ def generate_run_tests_portability_tests(name):
args = ["-l c -c dbg --build_only"],
docker_image_version = "tools/dockerfile/test/cxx_debian11_x86.current_version",
size = "enormous",
arch = "linux32",
)
test_names.append("runtests_c_linux_dbg_x86_build_only")

@ -1 +1 @@
us-docker.pkg.dev/grpc-testing/testing-images-public/cxx_debian11_x86:75a2427fa130db1a68fdbe0f4ae391e57b77504f@sha256:3f505ad99e52a4b3337fedb413e883bc8e5c1d9c089299c34002b89e01254d3b
us-docker.pkg.dev/grpc-testing/testing-images-public/cxx_debian11_x86:2394059ad8097ad28f11e24d81459e157c8d219b@sha256:43d6e6d97740a8f989024939b5507ed45071d00171eb68e1fbf101a9352860b1

@ -118,5 +118,7 @@ RUN curl -sSL -o ccache.tar.gz https://github.com/ccache/ccache/releases/downloa
RUN mkdir /var/local/jenkins
# Define the default command.
CMD ["bash"]
# docker is running on a 64-bit machine, so we need to
# override "uname -m" to report i686 instead of x86_64, otherwise
# cmake will choose a wrong option to build.
ENTRYPOINT ["linux32"]

Loading…
Cancel
Save