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

Use standard CMake options to specify C/C++ language standard, add BoringSSL to speed test #1410

Merged
merged 6 commits into from
Jan 23, 2024

Conversation

andrewhop
Copy link
Contributor

Issues:

Resolves CryptoAlg-2257 CryptoAlg-1208

Description of changes:

  1. Instead of specifying the direct C/C++ flags for language standard use the CMAKE_C_STANDARD option
  2. Default to the same c99/c++11 but let AWS-LC builders override it
  3. Update tool's CMakeList to search for libdecript and link it to any benchmark if found (needed for AES XTS and BoringSSL)
  4. Add BoringSSL to the speed sanity test
  5. Update the speed sanity test to build with C11 and C++14 (needed for BoringSSL which uses C11 features in their header files)

Call-outs:

BoringSSL already migrated to using CMAKE_C_STANDARD which we initially skipped because they also bumped up to C11 while we want to maintain C99 support.

Testing:

Built locally. Looking at the build out I verified the default and optional override are being set as expected:

  1. cmake -GNinja
  2. build.ninja contains -std=gnu99 and -std=gnu++11

With optional override:

  1. cmake -GNinja -DCMAKE_CXX_STANDARD=14 -DCMAKE_C_STANDARD=11
  2. build.ninja contains -std=gnu11 and -std=gnu++14

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

…AWS-LC builders to override the default c99/c++11 option. Add BoringSSL to the benchmark test run
@andrewhop andrewhop requested a review from a team as a code owner January 18, 2024 21:16
@codecov-commenter
Copy link

codecov-commenter commented Jan 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (71d119b) 76.84% compared to head (d9e06da) 76.81%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1410      +/-   ##
==========================================
- Coverage   76.84%   76.81%   -0.04%     
==========================================
  Files         425      425              
  Lines       71502    71502              
==========================================
- Hits        54948    54926      -22     
- Misses      16554    16576      +22     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@andrewhop andrewhop merged commit aab14cf into aws:main Jan 23, 2024
35 checks passed
dougch pushed a commit to dougch/aws-lc that referenced this pull request Jan 30, 2024
…ringSSL to speed test (aws#1410)

* Use standard CMake options to specify C/C++ language standard. Allow AWS-LC builders to override the default c99/c++11 option. Add BoringSSL to the benchmark test run

* Turn off secp256k1 when speed is built with BoringSSL
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants