[compiler-rt][builtins][RISCV] Always include __mul[sd]i3 builtin definitions

[luismarques]

Summary

The RISC-V implementations of the __mulsi3, __muldi3 builtins were conditionally compiling the actual function definitions depending on whether the M extension was present or not. This caused Compiler-RT testing failures for RISC-V targets with the M extension, as when these sources were included the librt_has_mul*i3 features were still being defined. These librt_has_* definitions are used to conditionally run the respective tests. Since the actual functions were not being compiled-in, the generic test for __muldi3 would fail. This patch makes these implementations follow the normal Compiler-RT convention of always including the definition, and conditionally running the respective tests by using the lit conditional REQUIRES: librt_has_*.

Since the mulsi3_test.c wasn't actually RISC-V-specific, this patch also moves it out of the riscv directory. It now only depends on librt_has_mulsi3 to run.

[lenary]

Two nits about the implementations. Otherwise, LGTM!

compiler-rt/lib/builtins/riscv/int_mul_impl.inc

12–16

I wonder if this file should have a stanza at the top something like the following (which should make it clearer how this file is used):

#ifndef __mulxi3
#error "__mulxi3 must be defined to use this generic implementation"
#endif

compiler-rt/test/builtins/Unit/mulsi3_test.c

9

I went looking for the file this comment references and could not find it. The only file with this filename is the current file?!

Anyway this is all to say: this comment was confusing and thanks for deleting it.

18

I *think* you should be using x here rather than re-calling __mulsi3.

[luismarques]

compiler-rt/test/builtins/Unit/mulsi3_test.c

9

Indeed. Before I submitted this patch I checked in the git history if such a file existed when this test was originally added, and it did not. I think it was supposed to say it was based on muldi3_test.c instead. It's probably best to just remove it. I imagine that people who care about this file will also know to look at muldi3_test.c, especially now that this patch moves this test to a target-independent location.

[luismarques]

Address review concerns.

[luismarques]

Ping.

[asb]

This LGTM. @lenary are you happy your comments are addressed?

[lenary]

LGTM. Yes you got all my changes.

compiler-rt/lib/builtins/riscv/int_mul_impl.inc

 //===-- int_mul_impl.inc - Integer multiplication -------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Helpers used by __mulsi3, __muldi3.
 //
 //===----------------------------------------------------------------------===//
 
-#if !defined(__riscv_mul)
+#ifndef __mulxi3
+#error "__mulxi3 must be defined to use this generic implementation"
+#endif
+
   .text
   .align 2
 
   .globl __mulxi3
   .type  __mulxi3, @function
 __mulxi3:
   mv     a2, a0
   mv     a0, zero
 .L1:
   andi   a3, a1, 1
   beqz   a3, .L2
   add    a0, a0, a2
 .L2:
   srli   a1, a1, 1
   slli   a2, a2, 1
   bnez   a1, .L1
   ret
-#endif

compiler-rt/test/builtins/Unit/muldi3_test.c

 // RUN: %clang_builtins %s %librt -o %t && %run %t
 // REQUIRES: librt_has_muldi3
 
 #include "int_lib.h"
 #include <stdio.h>
 
 COMPILER_RT_ABI di_int __muldi3(di_int a, di_int b);
 
 int test__muldi3(di_int a, di_int b, di_int expected)
 {
     di_int x = __muldi3(a, b);
     if (x != expected)
         printf("error in __muldi3: %lld * %lld = %lld, expected %lld\n",
-               a, b, __muldi3(a, b), expected);
+               a, b, x, expected);
     return x != expected;
 }
 
 char assumption_1[sizeof(di_int) == 2*sizeof(si_int)] = {0};
 
 int main()
 {
     if (test__muldi3(0, 0, 0))

compiler-rt/test/builtins/Unit/mulsi3_test.c

-// REQUIRES: riscv32-target-arch
 // RUN: %clang_builtins %s %librt -o %t && %run %t
+// REQUIRES: librt_has_mulsi3
 
 #include "int_lib.h"
 #include <stdio.h>
 #include <limits.h>
 
-#if !defined(__riscv_mul) && __riscv_xlen == 32
-// Based on mulsi3_test.c
-
 COMPILER_RT_ABI si_int __mulsi3(si_int a, si_int b);
 
 int test__mulsi3(si_int a, si_int b, si_int expected)
 {
     si_int x = __mulsi3(a, b);
     if (x != expected)
         printf("error in __mulsi3: %d * %d = %d, expected %d\n",
-               a, b, __mulsi3(a, b), expected);
+               a, b, x, expected);
     return x != expected;
 }
-#endif
 
 int main()
 {
-#if !defined(__riscv_mul) && __riscv_xlen == 32
     if (test__mulsi3(0, 0, 0))
         return 1;
     if (test__mulsi3(0, 1, 0))
         return 1;
     if (test__mulsi3(1, 0, 0))
         return 1;
     if (test__mulsi3(0, 10, 0))
         return 1;
int main()
     if (test__mulsi3(8192, 4194303, 34359730176))
         return 1;
     if (test__mulsi3(-8192, 4194303, -34359730176))
         return 1;
     if (test__mulsi3(8192, -4194303, -34359730176))
         return 1;
     if (test__mulsi3(-8192, -4194303, 34359730176))
         return 1;
-#else
-    printf("skipped\n");
-#endif
 
     return 0;
 }