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

[benshi001]

clang/lib/Basic/Targets/AVR.cpp

385

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

__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

__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

I will fix that when committing.

clang/lib/Basic/Targets/AVR.cpp

 
 void AVRTargetInfo::getTargetDefines(const LangOptions &Opts,
                                      MacroBuilder &Builder) const {
   Builder.defineMacro("AVR");
   Builder.defineMacro("__AVR");
   Builder.defineMacro("__AVR__");
   Builder.defineMacro("__ELF__");
 
+  if (ABI == "avrtiny")
+    Builder.defineMacro("__AVR_TINY__", "1");
+
   if (!this->CPU.empty()) {
     auto It = llvm::find_if(
         AVRMcus, [&](const MCUInfo &Info) { return Info.Name == this->CPU; });
 
-    if (It->IsTiny)
-      Builder.defineMacro("__AVR_TINY__", "1");
-
     if (It != std::end(AVRMcus)) {
       Builder.defineMacro(It->DefineName);
       if (It->NumFlashBanks >= 1)
         Builder.defineMacro("__flash", "__attribute__((address_space(1)))");
       if (It->NumFlashBanks >= 2)
         Builder.defineMacro("__flash1", "__attribute__((address_space(2)))");
       if (It->NumFlashBanks >= 3)
         Builder.defineMacro("__flash2", "__attribute__((address_space(3)))");

clang/test/Preprocessor/avr-common.c

 // RUN: %clang_cc1 -E -dM -triple avr-unknown-unknown -target-cpu attiny13 /dev/null | FileCheck -match-full-lines --check-prefixes=CHECK,AVR %s
+// RUN: %clang_cc1 -E -dM -triple avr-unknown-unknown -target-cpu avr25 /dev/null | FileCheck -match-full-lines --check-prefixes=CHECK,AVR %s
 // RUN: %clang_cc1 -E -dM -triple avr-unknown-unknown -target-cpu attiny4 /dev/null | FileCheck -match-full-lines --check-prefixes=CHECK,TINY %s
+// RUN: %clang_cc1 -E -dM -triple avr-unknown-unknown -target-cpu avrtiny /dev/null | FileCheck -match-full-lines --check-prefixes=CHECK,TINY %s
 
 // CHECK: #define AVR 1
 // CHECK: #define __AVR 1
 
 // TINY: #define __AVR_TINY__ 1
 // AVR-NOT: __AVR_TINY__
 
 // CHECK: #define __AVR__ 1
 // CHECK: #define __ELF__ 1

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

 //     if (A & 1)
 //       S += B;
 //     A = ((unsigned int) A) >> 1;
 //     B <<= 1;
 //   }
 //   return S;
 // }
 //
+// __mulhi3 has special ABI, as the implementation of libgcc, R25:R24 is used
+// to return result, while Rtmp/R21/R22/R23 are clobbered.
+//
 //===----------------------------------------------------------------------===//
 
   .text
   .align 2
 
+#ifdef __AVR_TINY__
+  .set __tmp_reg__, 16
+  .set __zero_reg__, 17
+#else
+  .set __tmp_reg__, 0
+  .set __zero_reg__, 1
+#endif
+
   .globl __mulhi3
   .type  __mulhi3, @function
 
 __mulhi3:
-  eor    r28, r28
-  eor    r20, r20
-  eor    r21, r21         ; Initialize the result to 0: `S = 0;`.
+  ; Use Rzero:Rtmp to store the result.
+  clr   __tmp_reg__
+  clr   __zero_reg__                 ; S = 0;
 
 __mulhi3_loop:
-  cp     r24, r28
-  cpc    r25, r28         ; `while (A != 0) { ... }`
-  breq   __mulhi3_end     ; End the loop if A is 0.
-
-  mov    r29, r24
-  andi   r29, 1           ; `if (A & 1) { ... }`
-  breq   __mulhi3_loop_a  ; Omit the accumulation (`S += B;`) if  A's LSB is 0.
-
-  add    r20, r22
-  adc    r21, r23         ; Do the accumulation: `S += B;`.
+  clr   r21
+  cp    r24, r21
+  cpc   r25, r21
+  breq  __mulhi3_end                 ; while (A != 0) {
+
+  mov   r21, r24
+  andi  r21, 1
+  breq  __mulhi3_loop_a              ;   if (A & 1)
+  add   __tmp_reg__, r22
+  adc   __zero_reg__, r23            ;     S += B;
 
 __mulhi3_loop_a:
-  lsr    r25
-  ror    r24              ; `A = ((unsigned int) A) >> 1;`.
-  lsl    r22
-  rol    r23              ; `B <<= 1;`
-
-  rjmp   __mulhi3_loop
+  lsr   r25
+  ror   r24                          ;   A = ((unsigned int) A) >> 1;
+  lsl   r22
+  rol   r23                          ;   B <<= 1;
+  rjmp  __mulhi3_loop                ; }
 
 __mulhi3_end:
-  mov    r24, r20
-  mov    r25, r21
-  ret
+  ; Return the result via R25:R24.
+  mov   r24, __tmp_reg__
+  mov   r25, __zero_reg__
+  ; Restore __zero_reg__ to 0.
+  clr   __zero_reg__
+  ret                                ; return S;

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
+  clr   __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;