Page MenuHomePhabricator

[compiler-rt][builtins] Add helper functions for uint16/sint16/uint8/sint8 div&mod
ClosedPublic

Authored by benshi001 on Thu, Apr 28, 2:02 AM.

Details

Summary
__udivmodhi4: uint16 div and mod
__udivmodqi4: uint8 div and mod
__divmodhi4 : sint16 div and mod
__divmodqi4 : sint8 div and mod

The above helper functions in libgcc have special ABI as described at
https://gcc.gnu.org/wiki/avr-gcc#Exceptions_to_the_Calling_Convention .

Diff Detail

Event Timeline

benshi001 created this revision.Thu, Apr 28, 2:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptThu, Apr 28, 2:02 AM
benshi001 requested review of this revision.Thu, Apr 28, 2:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptThu, Apr 28, 2:02 AM
Herald added subscribers: Restricted Project, jacquesguan. · View Herald Transcript
benshi001 updated this revision to Diff 425998.Fri, Apr 29, 2:23 AM
benshi001 edited the summary of this revision. (Show Details)
benshi001 retitled this revision from [compiler-rt][builtins] Add helper functions for uint16/sint16/uint8/sint8 div & mod to [compiler-rt][builtins] Add helper functions for uint16/sint16/uint8 div & mod.
benshi001 edited the summary of this revision. (Show Details)
benshi001 set the repository for this revision to rG LLVM Github Monorepo.

Some key points

  1. All helper functions are written in the minimal AVR instruction set, so they can run on all devices.
  2. Currently llvm generates call to __divmodhi4 for int8 div/mod, so we omit __divmodqi4.
  3. They can be built for different device families via specifying CMAKE_CXX_FLAGS and CMAKE_C_FLAGS when doing cmake.
  4. They have all been tested with random numbers, and all run correctly. divmodhi4: https://github.com/benshi001/avr-test/blob/main/divmod_sint16.ino udivmodhi4: https://github.com/benshi001/avr-test/blob/main/divmod_uint16.ino udivmodqi4: https://github.com/benshi001/avr-test/blob/main/divmod_uint8.ino
  5. The implementation of the unsigned ones are easy to understand, but divmodhi4 is more tricky, it has sub functions and even calls __udivmodhi4. So it is hard to make a clear decription comment among the source code lines.
  6. I am quite confident they are correct since they do work on my AVR board, my final goal is making "clang+llvm+compiler-rt+lld" fully functional and can replace gnu tool chain.
benshi001 retitled this revision from [compiler-rt][builtins] Add helper functions for uint16/sint16/uint8 div & mod to [compiler-rt][builtins] Add helper functions for uint16/sint16/uint8/sint8 div and mod.Fri, Apr 29, 2:42 AM
benshi001 edited the summary of this revision. (Show Details)
aykevl added a comment.Wed, May 4, 2:09 PM

Looks good to me.

It would be nice to write these functions in C instead of in assembly (for easier debugging and improved code generation for more advanced chips) but the special calling convention makes this difficult.

A future improvement could be to implement this special calling convention in the AVR backend of LLVM, so that LLVM can generate more efficient code. But I don't think it's necessary right now.

compiler-rt/lib/builtins/avr/divmodhi4.S
22–26

Can you use __tmp_reg__ instead of manually defining rtmp?

aykevl added a comment.Wed, May 4, 5:26 PM

The following code produces __divmodqi4, so I think it is still needed:

define i8 @divqi4(i8 %a, i8 %b) addrspace(1) {
  %result = sdiv i8 %a, %b
  ret i8 %result
}

See: https://llvm.godbolt.org/z/nTbcTfbb7

benshi001 updated this revision to Diff 427279.Thu, May 5, 5:01 AM
benshi001 retitled this revision from [compiler-rt][builtins] Add helper functions for uint16/sint16/uint8/sint8 div and mod to [compiler-rt][builtins] Add helper functions for uint16/sint16/uint8/sint8 div&mod.
benshi001 edited the summary of this revision. (Show Details)
benshi001 marked an inline comment as done.Thu, May 5, 5:04 AM

The following code produces __divmodqi4, so I think it is still needed:

define i8 @divqi4(i8 %a, i8 %b) addrspace(1) {
  %result = sdiv i8 %a, %b
  ret i8 %result
}

See: https://llvm.godbolt.org/z/nTbcTfbb7

I have implemented __divmodqi4. Here are the contrast tests:

divmodhi4: https://github.com/benshi001/avr-test/blob/main/divmod_sint16.ino

divmodqi4: https://github.com/benshi001/avr-test/blob/main/divmod_sint8.ino

udivmodhi4: https://github.com/benshi001/avr-test/blob/main/divmod_uint16.ino

udivmodqi4: https://github.com/benshi001/avr-test/blob/main/divmod_uint8.ino

compiler-rt/lib/builtins/avr/divmodhi4.S
22–26

changed to __tmp_reg__

aykevl accepted this revision.Thu, May 5, 8:20 AM

Looks good to me.

compiler-rt/lib/builtins/avr/divmodhi4.S
22–26

No I mean, can you remove this .set __tmp_reg__, num entirely? Since D119807 this should not be needed anymore. Or do you want to keep it for backwards compatibility with Clang 14?

This revision is now accepted and ready to land.Thu, May 5, 8:20 AM
benshi001 marked an inline comment as done.Thu, May 5, 4:27 PM
benshi001 added inline comments.
compiler-rt/lib/builtins/avr/divmodhi4.S
22–26

I expect they can be built with older version of clang. :)

brad added a subscriber: koakuma.Fri, May 13, 9:37 PM