This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by benshi001 on May 6 2022, 12:25 AM.

Diff Detail

Event Timeline

benshi001 created this revision.May 6 2022, 12:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 6 2022, 12:25 AM
Herald added subscribers: Jim, dberris. · View Herald Transcript
benshi001 requested review of this revision.May 6 2022, 12:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 6 2022, 12:25 AM
Herald added subscribers: Restricted Project, jacquesguan. · View Herald Transcript
benshi001 updated this revision to Diff 427601.May 6 2022, 5:03 AM
benshi001 added inline comments.May 6 2022, 5:06 AM
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.

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

benshi001 added inline comments.May 6 2022, 5:10 AM
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.

aykevl added a comment.May 6 2022, 6:23 AM

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.

aykevl accepted this revision.May 6 2022, 6:23 AM
This revision is now accepted and ready to land.May 6 2022, 6:23 AM
benshi001 marked an inline comment as done.May 6 2022, 6:40 AM

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.

This revision was automatically updated to reflect the committed changes.
benshi001 marked an inline comment as done.
Herald added a project: Restricted Project. · View Herald TranscriptMay 6 2022, 6:47 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript