int_types.h: rv32: check for __SIZEOF_INT128__ on riscv

[xobs]

Summary

When building libstd on Rust for a riscv32 target, compiler-rt fails to build with the following error:

running: "riscv-none-elf-gcc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-march=rv32imac" "-mabi=ilp32" "-mcmodel=medany" "-fno-builtin" "-fvisibility=hidden" "-ffreestanding" "-fomit-frame-pointer" "-ffile-prefix-map=E:\\Code\\Xous\\rust-next\\src\\llvm-project\\compiler-rt=." "-DVISIBILITY_HIDDEN" "-o" "E:\\Code\\Xous\\rust-next\\target\\riscv32imac-unknown-xous-elf\\release\\build\\compiler_builtins-b0d7dd25c6999904\\out\\absvdi2.o" "-c" "E:\\Code\\Xous\\rust-next\\src\\llvm-project\\compiler-rt\\lib/builtins\\absvdi2.c"
cargo:warning=In file included from E:\Code\Xous\rust-next\src\llvm-project\compiler-rt\lib/builtins\int_lib.h:99,
cargo:warning=                 from E:\Code\Xous\rust-next\src\llvm-project\compiler-rt\lib/builtins\absvdi2.c:13:
cargo:warning=E:\Code\Xous\rust-next\src\llvm-project\compiler-rt\lib/builtins\int_types.h:79:1: error: unable to emulate 'TI'
cargo:warning=   79 | typedef int ti_int __attribute__((mode(TI)));
cargo:warning=      | ^~~~~~~
cargo:warning=E:\Code\Xous\rust-next\src\llvm-project\compiler-rt\lib/builtins\int_types.h:80:1: error: unable to emulate 'TI'
cargo:warning=   80 | typedef unsigned tu_int __attribute__((mode(TI)));
cargo:warning=      | ^~~~~~~
exit code: 1

This appears to be because 128-bit support is gated on the __riscv compiler macro which is valid for both rv32 and rv64. However, only rv64 has 128-bit support, so this fails when building for rv32.

Add a check for __SIZEOF_INT128__ to ensure that 128-bit support is only enabled on targets that support it.

[xobs]

This diff had its line endings changed from dos to unix.

[arichardson]

compiler-rt/lib/builtins/int_types.h

67

Can this be simplified to defined(__SIZEOF_INT128__) || defined(_WIN64)?

[MaskRay]

arichardson' defined(__SIZEOF_INT128__) suggestions LGTM.

[xobs]

Remove __riscv check entirely and rely on the presence of __SIZEOF_INT128__, which is set for rv64 and not for rv32.

[MaskRay]

Looks great!

[xobs]

Fantastic! How do we get this landed now? Can you please recommend someone for that?

[MaskRay]

In D137485#3910656, @xobs wrote:

Fantastic! How do we get this landed now? Can you please recommend someone for that?

You can provide your name and email and I will do git commit --amend --author='...' and land this on your behalf :)

If you set up arcanist, use the following patch, and upload the patch with arc diff 'HEAD^', your name/email will be recorded on this Differential:)

--- i/src/workflow/ArcanistDiffWorkflow.php
+++ w/src/workflow/ArcanistDiffWorkflow.php
@@ -2361,7 +2361,7 @@ EOTEXT

     // If we track an upstream branch either directly or indirectly, use that.
     $branch = $api->getBranchName();
-    if (strlen($branch)) {
+    if (phutil_nonempty_string($branch)) {
       $upstream_path = $api->getPathToUpstream($branch);
       $remote_branch = $upstream_path->getRemoteBranchName();
       if ($remote_branch !== null) {

[xobs]

Here's the contents of the patch from my local git repo, which includes my name and email address:

commit 96b02138e389855ad0dd3b2a165b2a20ca294d55 (HEAD -> riscv-fix)
Author: Sean Cross <[e-mail]>
Date:   Sat Nov 5 17:56:48 2022 +0800

    [compiler-rt] disable TI on 32-bit riscv

    The macro `CRT_HAS_128BIT` is currently gated behind the `__riscv`
    macro. This macro is present on riscv32 as well as riscv64, however
    riscv32 does not have support for TI mode.

    Replace this test with a check for the macro `__SIZEOF_INT128__`. This
    effectively prevents this from being enabled on riscv targets where
    there is no support for int128.

    This fixes github issue #58826.

    Signed-off-by: Sean Cross <[e-mail]>

diff --git a/compiler-rt/lib/builtins/int_types.h b/compiler-rt/lib/builtins/int_types.h
index 7a72de480676..e94d3154c6d4 100644
--- a/compiler-rt/lib/builtins/int_types.h
+++ b/compiler-rt/lib/builtins/int_types.h
@@ -64,7 +64,7 @@ typedef union {
 } udwords;

 #if defined(__LP64__) || defined(__wasm__) || defined(__mips64) ||             \
-    defined(__riscv) || defined(_WIN64)
+    defined(__SIZEOF_INT128__) || defined(_WIN64)
 #define CRT_HAS_128BIT
 #endif

I'm afraid I don't have Arcanist set up yet because I'm on Windows and its installation documentation for Windows is somewhat sparse.

compiler-rt/lib/builtins/int_types.h

 #else
     su_int high;
     su_int low;
 #endif // _YUGA_LITTLE_ENDIAN
   } s;
 } udwords;
 
 #if defined(__LP64__) || defined(__wasm__) || defined(__mips64) ||             \
-    defined(__riscv) || defined(_WIN64)
+    defined(__SIZEOF_INT128__) || defined(_WIN64)
 #define CRT_HAS_128BIT
 #endif
 
 // MSVC doesn't have a working 128bit integer type. Users should really compile
 // compiler-rt with clang, but if they happen to be doing a standalone build for
 // asan or something else, disable the 128 bit parts so things sort of work.
 #if defined(_MSC_VER) && !defined(__clang__)
 #undef CRT_HAS_128BIT