[compiler-rt][builtins] Fix wrong ABI of AVR __mulqi3 & __mulhi3

[benshi001]

clang/lib/Basic/Targets/AVR.cpp

385 ↗

(On Diff #427601)

The previous way of definition of __AVR_TINY__ is incorrect, it works for -mmcu=<any tiny mcu name>, but does not work if -mmcu=avrtiny is specified.

Since this is just a minor fix, I won't make another patch.

[benshi001]

The new implementation of __mulhi3 and __mulqi3 strictly follow libgcc's special ABI.

[benshi001]

compiler-rt/lib/builtins/avr/mulhi3.S

44 ↗

(On Diff #427601)

__zero_reg__ is used to store the result and is clobbered. So we have to restore it to zero value (eor __zero_reg__, __zero_reg__) before return.

[benshi001]

Corresponding tests

__mulhi3 : https://github.com/benshi001/avr-test/blob/main/mul_int16.ino
__mulqi3 : https://github.com/benshi001/avr-test/blob/main/mul_int8.ino

[aykevl]

Looks good to me. It might be possible to optimize these functions a bit more but that can happen at a later time.

compiler-rt/lib/builtins/avr/mulhi3.S

44 ↗

(On Diff #427601)

__zero_reg__ can be assumed to be zero on function entry so it doesn't have to be cleared here. Both LLVM and avr-gcc use it as a reserved register.

Also, writing clr Rd is maybe easier to read than eor Rd, Rd? It is an alias and results in the same machine code.

[benshi001]

In D125077#3496592, @aykevl wrote:

Looks good to me. It might be possible to optimize these functions a bit more but that can happen at a later time.

Yes. They are far from perfect. We can optimize them according different instruction set, such as using MOVW for int16 copy.

compiler-rt/lib/builtins/avr/mulhi3.S

44 ↗

(On Diff #427601)

I will fix that when committing.

compiler-rt/lib/builtins/avr/mulqi3.S

 //===------------ mulhi3.S - int8 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
 //
 //===----------------------------------------------------------------------===//
 //
 // The corresponding C code is something like:
 //
-// int __mulqi3(char A, char B) {
-//   return __mulhi3((int) A, (int) B);
-// }
+// char __mulqi3(char A, char B) {
+//   int S = 0;
+//   while (A != 0) {
+//     if (A & 1)
+//       S += B;
+//     B <<= 1;
+//     A = ((unsigned char) A) >> 1;
+//   }
+//   return S;
+// }
+//
+// __mulqi3 has special ABI, as the implementation of libgcc, the result is
+// returned via R24, while Rtmp and R22 are clobbered.
 //
 //===----------------------------------------------------------------------===//
 
   .text
   .align 2
 
+#ifdef __AVR_TINY__
+  .set __tmp_reg__, 16
+#else
+  .set __tmp_reg__, 0
+#endif
+
   .globl __mulqi3
   .type  __mulqi3, @function
 
 __mulqi3:
-  mov    r25, r24
-  lsl    r25
-  sbc    r25, r25         ; Promote A from char to int: `(int) A`.
-  mov    r23, r22
-  lsl    r23
-  sbc    r23, r23         ; Promote B from char to int: `(int) B`.
-  rcall  __mulhi3         ; `__mulhi3((int) A, (int) B);`.
-  ret
+  eor   __tmp_reg__, __tmp_reg__ ; S = 0;
+
+__mulqi3_loop:
+  cpi   r24, 0
+  breq  __mulqi3_end             ; while (A != 0) {
+  sbrc  r24, 0                   ;   if (A & 1)
+  add   __tmp_reg__, r22         ;     S += B;
+  add   r22, r22                 ;   B <<= 1;
+  lsr   r24                      ;   A = ((unsigned char) A) >> 1;
+  rjmp  __mulqi3_loop            ; }
+
+__mulqi3_end:
+  mov   r24, __tmp_reg__
+  ret                            ; return S;