ring icon indicating copy to clipboard operation
ring copied to clipboard

Add support for arch csky

Open Dirreke opened this issue 1 year ago • 3 comments

Add support for arch csky. Ref: https://github.com/rust-lang/rust/blob/master/src/doc/rustc/src/platform-support/csky-unknown-linux-gnuabiv2.md

Dirreke avatar May 02 '24 06:05 Dirreke

@Dirreke Please:

  1. Make a mk/install-build-tools-unsupported.sh that is in the same form of mk/install-build-tools.sh, which adds support for installing the csky toolchain on an Ubuntu system, starting from the state where qemu-csky2 is not installed.
  2. Make a mk/cargo-unsupported.sh that mirrors mk/cargo.sh which uses the csky toolchain to run cargo commands in qemu-sky2.
  3. Update .github/workflows/ci.yml to add the csky target to the test matrix.

briansmith avatar May 18 '24 20:05 briansmith

Actually, I'm not sure if we should add this target to CI, because it's a Tier 3 target of rust and there's no prebuilt toolchain. We have to build rust/cargo from source (also including llvm/clang, because there's no prebuild llvm with csky target too), which will take lots of time. In addition, there are really few people using this target. Therefore, I think we can just keep it without CI and I will maintain it for the csky target if it's broken.

If you still insist that we need CI, please tell me and I will try to finish it.

Dirreke avatar May 20 '24 12:05 Dirreke

Actually, I'm not sure if we should add this target to CI, because it's a Tier 3 target of rust and there's no prebuilt toolchain. We have to build rust/cargo from source (also including llvm/clang, because there's no prebuild llvm with csky target too), which will take lots of time.

First, I don't think you'd need to build cargo from source because we'd be cross-compiling from host x86_64-unknown-linux-gnu. But, if it isn't even in LLVM yet then I agree that seems like quite a burden.

Here's an alternative: Change build.rs so that it contains some logic that adds a define for OPENSSL_32BIT if target_pointer_width = 32 or OPENSSL_64BIT if target_pointer_width = 64, replace this section that exablishes the allowlist for targets that we support that BoringSSL doesn't:

- #elif defined(__MIPSEL__) && !defined(__LP64__)
- #define OPENSSL_32_BIT
- #elif defined(__MIPSEL__) && defined(__LP64__)
- #define OPENSSL_64_BIT
- #elif defined(__MIPSEB__) && !defined(__LP64__)
- #define OPENSSL_32_BIT
- #elif defined(__PPC64__) || defined(__powerpc64__)
- #define OPENSSL_64_BIT
- #elif (defined(__PPC__) || defined(__powerpc__)) && defined(_BIG_ENDIAN)
- #define OPENSSL_32_BIT
- #elif defined(__s390x__)
- #define OPENSSL_64_BIT
+ #elif defined(OPENSSL_64_BIT) || defined(OPENSSL_32_BIT)
  #else
  #error "Unknown target CPU"
  #endif

And also add this to the bottom of the block of assertions in base.h:

#if defined(OPENSSL_64_BIT)
OPENSSL_STATIC_ASSERT(sizeof(size_t) == 8, "size_t isn't 64 bits on a 64-bit target.");
#endif
#if defined(OPENSSL_32_BIT)
OPENSSL_STATIC_ASSERT(sizeof(size_t) == 4, "size_t isn't 32 bits on 32-bit target.");
#endif

briansmith avatar May 20 '24 17:05 briansmith

Or how about using __WORDSIZE?

Dirreke avatar May 29 '24 12:05 Dirreke

Or how about using __WORDSIZE?

Based on https://bugs.chromium.org/p/nativeclient/issues/detail?id=1214, https://github.com/llvm/llvm-project/issues/48906, https://sourceware.org/bugzilla/show_bug.cgi?id=27574, and other things, I think __WORDSIZE isn't reliable.

Also, it seems to be part of the sysroot, so it isn't always defined. For example clang -dM -E -x c /dev/null doesn't output a definition for it.

I think there are not many 32-bit targets left to add. Since I merged #2082, I suggest you just add csky to the allowlist for 32-bit targets at the end. You don't need to do any other extra work. But it would be useful to see the output of clang --target=<your-target> -dM -E -x c /dev/null and the equivalent for GCC, in the commit messae.

briansmith avatar May 30 '24 03:05 briansmith

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.24%. Comparing base (ed5b2a8) to head (68c3309).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2042   +/-   ##
=======================================
  Coverage   97.24%   97.24%           
=======================================
  Files         144      144           
  Lines       19995    19995           
  Branches      228      228           
=======================================
  Hits        19444    19444           
  Misses        525      525           
  Partials       26       26           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar May 31 '24 16:05 codecov[bot]