[PATCH] D123200: [compiler-rt][builtins] Add several helper functions for AVR

[Ben Shi via Phabricator via cfe-commits]

benshi001 added a comment.

In D123200#3495356 , @aykevl wrote:

> @benshi001 I have been looking through the GCC code and I think avr-gcc also 
> has a special calling convention for many other functions, including 
> `__mulqi3` and `__mulhi3`.
>
> Source:
>
> 1. I think this is where the ABI is specified in the compiler: 
> https://github.com/gcc-mirror/gcc/blob/releases/gcc-5.4.0/gcc/config/avr/avr.md#L1543-L1549
>  You can see that it multiplies R24 with R22 and stores the result in R24, 
> and clobbers R22 in the process. But no other registers.
> 2. In this code sample , avr-gcc doesn't 
> save `char c` (`r20`) across the `__mulqi3` and `__mulhi3` calls, which is 
> normally call-clobbered.
>
> Therefore, I think we need to be a bit more careful with defining these AVR 
> builtins and check the ABI in avr-gcc first.
> Also, we can make use of this and optimize the AVR backend more (you can see 
> that the Clang generated code is much worse than avr-gcc in the above 
> examples).

Let us discuss that at https://github.com/llvm/llvm-project/issues/55279


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123200/new/

https://reviews.llvm.org/D123200

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123200: [compiler-rt][builtins] Add several helper functions for AVR

[Ayke via Phabricator via cfe-commits]

aykevl added a comment.

@benshi001 I have been looking through the GCC code and I think avr-gcc also 
has a special calling convention for many other functions, including `__mulqi3` 
and `__mulhi3`.

Source:

1. I think this is where the ABI is specified in the compiler: 
https://github.com/gcc-mirror/gcc/blob/releases/gcc-5.4.0/gcc/config/avr/avr.md#L1543-L1549
 You can see that it multiplies R24 with R22 and stores the result in R24, and 
clobbers R22 in the process. But no other registers.
2. In this code sample , avr-gcc doesn't save 
`char c` (`r20`) across the `__mulqi3` and `__mulhi3` calls.

Therefore, I think we need to be a bit more careful with defining these AVR 
builtins and check the ABI in avr-gcc first.
Also, we can make use of this and optimize the AVR backend more (you can see 
that the Clang generated code is much worse than avr-gcc in the above examples).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123200/new/

https://reviews.llvm.org/D123200

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123200: [compiler-rt][builtins] Add several helper functions for AVR

[Ben Shi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
benshi001 marked an inline comment as done.
Closed by commit rGfb7a435492a5: [compiler-rt][builtins] Add several helper 
functions for AVR (authored by benshi001).

Changed prior to commit:
  https://reviews.llvm.org/D123200?vs=423805&id=426319#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123200/new/

https://reviews.llvm.org/D123200

Files:
  compiler-rt/cmake/Modules/CompilerRTUtils.cmake
  compiler-rt/cmake/base-config-ix.cmake
  compiler-rt/cmake/builtin-config-ix.cmake
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/builtins/CMakeLists.txt
  compiler-rt/lib/builtins/avr/exit.S
  compiler-rt/lib/builtins/avr/mulhi3.S
  compiler-rt/lib/builtins/avr/mulqi3.S

Index: compiler-rt/lib/builtins/avr/mulqi3.S
===
--- /dev/null
+++ compiler-rt/lib/builtins/avr/mulqi3.S
@@ -0,0 +1,31 @@
+//=== 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);
+// }
+//
+//===--===//
+
+	.text
+	.align 2
+
+	.globl __mulqi3
+	.type  __mulqi3, @function
+
+__mulqi3:
+	movr25, r24
+	lslr25
+	sbcr25, r25 ; Promote A from char to int: `(int) A`.
+	movr23, r22
+	lslr23
+	sbcr23, r23 ; Promote B from char to int: `(int) B`.
+	rcall  __mulhi3 ; `__mulhi3((int) A, (int) B);`.
+	ret
Index: compiler-rt/lib/builtins/avr/mulhi3.S
===
--- /dev/null
+++ compiler-rt/lib/builtins/avr/mulhi3.S
@@ -0,0 +1,58 @@
+//=== mulhi3.S - int16 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 __mulhi3(int A, int B) {
+//   int S = 0;
+//   while (A != 0) {
+// if (A & 1)
+//   S += B;
+// A = ((unsigned int) A) >> 1;
+// B <<= 1;
+//   }
+//   return S;
+// }
+//
+//===--===//
+
+	.text
+	.align 2
+
+	.globl __mulhi3
+	.type  __mulhi3, @function
+
+__mulhi3:
+	eorr28, r28
+	eorr20, r20
+	eorr21, r21 ; Initialize the result to 0: `S = 0;`.
+
+__mulhi3_loop:
+	cp r24, r28
+	cpcr25, r28 ; `while (A != 0) { ... }`
+	breq   __mulhi3_end ; End the loop if A is 0.
+
+	movr29, r24
+	andi   r29, 1   ; `if (A & 1) { ... }`
+	breq   __mulhi3_loop_a  ; Omit the accumulation (`S += B;`) if  A's LSB is 0.
+
+	addr20, r22
+	adcr21, r23 ; Do the accumulation: `S += B;`.
+
+__mulhi3_loop_a:
+	lsrr25
+	rorr24  ; `A = ((unsigned int) A) >> 1;`.
+	lslr22
+	rolr23  ; `B <<= 1;`
+
+	rjmp   __mulhi3_loop
+
+__mulhi3_end:
+	movr24, r20
+	movr25, r21
+	ret
Index: compiler-rt/lib/builtins/avr/exit.S
===
--- /dev/null
+++ compiler-rt/lib/builtins/avr/exit.S
@@ -0,0 +1,18 @@
+//=== exit.S - global terminator for AVR --===//
+//
+// 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
+//
+//===--===//
+
+	.text
+	.align 2
+
+	.globl _exit
+	.type  _exit, @function
+
+_exit:
+	cli ; Disable all interrupts.
+__stop_program:
+	rjmp __stop_program ; Fall into an infinite loop.
Index: compiler-rt/lib/builtins/CMakeLists.txt
===
--- compiler-rt/lib/builtins/CMakeLists.txt
+++ compiler-rt/lib/builtins/CMakeLists.txt
@@ -567,6 +567,13 @@
 set(armv8m.main_SOURCES ${arm_SOURCES})
 set(armv8.1m.main_SOURCES ${arm_SOURCES})
 
+# 8-bit AVR MCU
+set(avr_SOURCES
+  avr/mulqi3.S
+  avr/mulhi3.S
+  avr/exit.S
+)
+
 # hexagon arch
 set(hexagon_SOURCES
   hexagon/common_entry_exit_abi1.S
Index: compiler-rt/cmake/config-ix.cmake
===
--- compiler-rt/cmake/config-ix.cmake
+++ compiler-rt/cmak

[PATCH] D123200: [compiler-rt][builtins] Add several helper functions for AVR

[Ben Shi via Phabricator via cfe-commits]

benshi001 marked an inline comment as done.
benshi001 added inline comments.



Comment at: compiler-rt/lib/builtins/CMakeLists.txt:671
 
+set(avr_SOURCES
+  avr/mulqi3.S

MaskRay wrote:
> Keep the `*_SOURCES` in alphabetical order. Move avr to the beginning. Ignore 
> some entries which are unordered.
I will fix all your concerns about "alphabetical order" when committing.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123200/new/

https://reviews.llvm.org/D123200

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123200: [compiler-rt][builtins] Add several helper functions for AVR

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.



Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:171
   check_symbol_exists(__ve__ "" __VE)
+  check_symbol_exists(__AVR__ "" __AVR)
   if(__ARM)

Keep the list in alphabetical order. Move avr to the beginning. Ignore some 
entries which are unordered.



Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:216
 add_default_target_arch(ve)
+  elseif(__AVR)
+add_default_target_arch(avr)

Keep the list in alphabetical order. Move avr to the beginning. Ignore some 
entries which are unordered.



Comment at: compiler-rt/cmake/builtin-config-ix.cmake:55
 set(VE ve)
+set(AVR avr)
 

Keep the list in alphabetical order. Move avr to the beginning. Ignore some 
entries which are unordered.



Comment at: compiler-rt/lib/builtins/CMakeLists.txt:671
 
+set(avr_SOURCES
+  avr/mulqi3.S

Keep the `*_SOURCES` in alphabetical order. Move avr to the beginning. Ignore 
some entries which are unordered.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123200/new/

https://reviews.llvm.org/D123200

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits