Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Replace in-house rand with absl::Random for backoff" #26476

Merged
merged 1 commit into from Jun 15, 2021

Conversation

@veblush
Copy link
Contributor

veblush commented Jun 14, 2021

abseil/abseil-cpp#662 looks relevant to this

@veblush veblush added the release notes: no Indicates if PR should not be in release notes label Jun 14, 2021
@veblush
Copy link
Contributor

veblush commented Jun 14, 2021

Jan, do you have a different fusion dashboard for ARM builds than https://fusion.corp.google.com/label/1923284992 ?

@veblush
Copy link
Contributor

veblush commented Jun 14, 2021

It seems like we need to add absl::random specific configuration to makefile.

Except from abseil-cmake:

ABSL_RANDOM_HWAES_ARM32_FLAGS = [
    "-mfpu=neon",
]

ABSL_RANDOM_HWAES_ARM64_FLAGS = [
    "-march=armv8-a+crypto",
]


if(CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64|amd64|AMD64")
  if (MSVC)
    set(ABSL_RANDOM_RANDEN_COPTS "${ABSL_RANDOM_HWAES_MSVC_X64_FLAGS}")
  else()
    set(ABSL_RANDOM_RANDEN_COPTS "${ABSL_RANDOM_HWAES_X64_FLAGS}")
  endif()
elseif(CMAKE_SYSTEM_PROCESSOR MATCHES "arm.*|aarch64")
  if (CMAKE_SIZEOF_VOID_P STREQUAL "8")
    set(ABSL_RANDOM_RANDEN_COPTS "${ABSL_RANDOM_HWAES_ARM64_FLAGS}")
  elseif(CMAKE_SIZEOF_VOID_P STREQUAL "4")
    set(ABSL_RANDOM_RANDEN_COPTS "${ABSL_RANDOM_HWAES_ARM32_FLAGS}")
  else()
    message(WARNING "Value of CMAKE_SIZEOF_VOID_P (${CMAKE_SIZEOF_VOID_P}) is not supported.")
  endif()
else()
  message(WARNING "Value of CMAKE_SYSTEM_PROCESSOR (${CMAKE_SYSTEM_PROCESSOR}) is unknown and cannot be used to set ABSL_RANDOM_RANDEN_COPTS")
  set(ABSL_RANDOM_RANDEN_COPTS "")
endif()

@veblush
Copy link
Contributor

veblush commented Jun 14, 2021

Instead of rollback, we might handle this by #26479

@jtattermusch
Copy link
Contributor Author

Jan, do you have a different fusion dashboard for ARM builds than https://fusion.corp.google.com/label/1923284992 ?

The ARM builds weren't in the main dashboard before, but I just added them.
The impacted tests seem to have been fixed by the revert. Thanks!

@stanley-cheung stanley-cheung deleted the revert-26463-backoff branch July 28, 2021 17:36
stanhu added a commit to stanhu/grpc that referenced this pull request Aug 25, 2021
Older CPUs that do not have SSE4.1 would crash with the Ruby native gem
due to an illegal instruction exception.

The Abseil random library isn't being used at the moment
(grpc#26476), and there's no reason gRPC
needs to force SSE4.1 instructions on all platforms at the moment. There
are other hardware-specific issues that need to be ironed out for this
to work: grpc#27095

When the `-msse4` compiler flag was enabled, the Abseil code started
using the `pinsrb` instruction:

```
$ elfx86exts abseil-cpp/absl/time/internal/cctz/src/time_zone_libc.o
MODE64 (ret)
CMOV (cmovne)
SSE2 (movdqa)
SSE41 (pinsrb)
SSE1 (movaps)
CPU Generation: Penryn
```

Closes grpc#27095
stanhu added a commit to stanhu/grpc that referenced this pull request Aug 25, 2021
Older CPUs that do not have SSE4.1 would crash with the Ruby native gem
due to an illegal instruction exception.

The Abseil random library isn't being used at the moment
(grpc#26476), and there's no reason gRPC
needs to force SSE4.1 instructions on all platforms at the moment. There
are other hardware-specific issues that need to be ironed out for this
to work: grpc#27121

When the `-msse4` compiler flag was enabled, the Abseil code started
using the `pinsrb` instruction:

```
$ elfx86exts abseil-cpp/absl/time/internal/cctz/src/time_zone_libc.o
MODE64 (ret)
CMOV (cmovne)
SSE2 (movdqa)
SSE41 (pinsrb)
SSE1 (movaps)
CPU Generation: Penryn
```

Closes grpc#27095
stanhu added a commit to stanhu/grpc that referenced this pull request Aug 25, 2021
Older CPUs that do not have SSE4.1 would crash with the Ruby native gem
due to an illegal instruction exception.

The Abseil random library isn't being used at the moment
(grpc#26476), and there's no reason gRPC
needs to force SSE4.1 instructions on all platforms at the moment. There
are other hardware-specific issues that need to be ironed out for this
to work: grpc#27121

When the `-msse4` compiler flag was enabled, the Abseil code started
using the `pinsrb` instruction:

```
$ elfx86exts abseil-cpp/absl/time/internal/cctz/src/time_zone_libc.o
MODE64 (ret)
CMOV (cmovne)
SSE2 (movdqa)
SSE41 (pinsrb)
SSE1 (movaps)
CPU Generation: Penryn
```

Closes grpc#27095
stanhu added a commit to stanhu/grpc that referenced this pull request Aug 25, 2021
Older CPUs that do not have SSE4.1 would crash with the Ruby native gem
due to an illegal instruction exception.

The Abseil random library isn't being used at the moment
(grpc#26476), and there's no reason gRPC
needs to force SSE4.1 instructions on all platforms at the moment. There
are other hardware-specific issues that need to be ironed out for this
to work: grpc#26479

When the `-msse4` compiler flag was enabled, the Abseil code started
using the `pinsrb` instruction:

```
$ elfx86exts abseil-cpp/absl/time/internal/cctz/src/time_zone_libc.o
MODE64 (ret)
CMOV (cmovne)
SSE2 (movdqa)
SSE41 (pinsrb)
SSE1 (movaps)
CPU Generation: Penryn
```

Closes grpc#27095
stanhu added a commit to stanhu/grpc that referenced this pull request Aug 25, 2021
Older CPUs that do not have SSE4.1 would crash with the Ruby native gem
due to an illegal instruction exception.

The Abseil random library isn't being used at the moment
(grpc#26476), and there's no reason gRPC
needs to force SSE4.1 instructions on all platforms at the moment. There
are other hardware-specific issues that need to be ironed out for this
to work: grpc#26479

When the `-msse4` compiler flag was enabled, the Abseil code started
using the `pinsrb` instruction:

```
$ elfx86exts abseil-cpp/absl/time/internal/cctz/src/time_zone_libc.o
MODE64 (ret)
CMOV (cmovne)
SSE2 (movdqa)
SSE41 (pinsrb)
SSE1 (movaps)
CPU Generation: Penryn
```

Closes grpc#27095
stanhu added a commit to stanhu/grpc that referenced this pull request Sep 10, 2021
Older CPUs that do not have SSE4.1 would crash with the Ruby native gem
due to an illegal instruction exception.

The Abseil random library isn't being used at the moment
(grpc#26476), and there's no reason gRPC
needs to force SSE4.1 instructions on all platforms at the moment. There
are other hardware-specific issues that need to be ironed out for this
to work: grpc#26479

When the `-msse4` compiler flag was enabled, the Abseil code started
using the `pinsrb` instruction:

```
$ elfx86exts abseil-cpp/absl/time/internal/cctz/src/time_zone_libc.o
MODE64 (ret)
CMOV (cmovne)
SSE2 (movdqa)
SSE41 (pinsrb)
SSE1 (movaps)
CPU Generation: Penryn
```

This was previously needed because gcc 4.8 wouldn't compile without the
`-msse4` and `-maes` flags.

However, per
gcc-mirror/gcc@97db2bf
gcc 5.0+ automatically detects whether these options are enabled.

clang still needs `-maes` since including `wmmintrin.h` expects the AES
option to be enabled.

Closes grpc#27095
jtattermusch pushed a commit that referenced this pull request Oct 5, 2021
* Drop ABSL_RANDOM_HWAES_FLAGS compiler flags

Older CPUs that do not have SSE4.1 would crash with the Ruby native gem
due to an illegal instruction exception.

The Abseil random library isn't being used at the moment
(#26476), and there's no reason gRPC
needs to force SSE4.1 instructions on all platforms at the moment. There
are other hardware-specific issues that need to be ironed out for this
to work: #26479

When the `-msse4` compiler flag was enabled, the Abseil code started
using the `pinsrb` instruction:

```
$ elfx86exts abseil-cpp/absl/time/internal/cctz/src/time_zone_libc.o
MODE64 (ret)
CMOV (cmovne)
SSE2 (movdqa)
SSE41 (pinsrb)
SSE1 (movaps)
CPU Generation: Penryn
```

Closes #27095

* Revert "Drop ABSL_RANDOM_HWAES_FLAGS compiler flags"

This reverts commit 3b7cc74.

* Drop -msse4 compiler flag

Older CPUs that do not have SSE4.1 would crash with the Ruby native gem
due to an illegal instruction exception.

The Abseil random library isn't being used at the moment
(#26476), and there's no reason gRPC
needs to force SSE4.1 instructions on all platforms at the moment. There
are other hardware-specific issues that need to be ironed out for this
to work: #26479

When the `-msse4` compiler flag was enabled, the Abseil code started
using the `pinsrb` instruction:

```
$ elfx86exts abseil-cpp/absl/time/internal/cctz/src/time_zone_libc.o
MODE64 (ret)
CMOV (cmovne)
SSE2 (movdqa)
SSE41 (pinsrb)
SSE1 (movaps)
CPU Generation: Penryn
```

This was previously needed because gcc 4.8 wouldn't compile without the
`-msse4` and `-maes` flags.

However, per
gcc-mirror/gcc@97db2bf
gcc 5.0+ automatically detects whether these options are enabled.

clang still needs `-maes` since including `wmmintrin.h` expects the AES
option to be enabled.

Closes #27095
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: no Indicates if PR should not be in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants