This file is also linked to libclang.a.

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

@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.

Looks good to me.

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

Looks good to me.

Looks good to me.

This change resulted in a regression for AVR: D124939

ping

Not a real review but I found this which looks like a bug.

What about chip families other than avr2 (the default)? Some have a different PC pointer size (1-3 bytes, therefore affecting the ABI), some have different instructions, etc.

@MaskRay it was my suggestion to move this from the toolchain specific file to the generic file, because it makes the implementation much simpler. See my comment D117423#3251110 for details.

@benshi001 I've rebased the patch and updated it with your suggestions.

Hmm, ok. An assembly test might have been useful to show that it actually works but the patch looks fine to me so I'll accept it.

I assume you've verified the new test cases with avr-gcc? If so, this looks good to me.

I think there is one issue in the test. Other than that, this looks good to me.

Looks good to me!

This also affects Cortex-M0, but I don't think __sync_* routines actually exist in any Cortex-M0 libraries. So in practice this just leads to a slightly different linker error for those cases, I think.

Ping?

Ok, I've updated this patch to remove EnableHeaderDuplication.

• fix

• add Go API

@mehdi_amini thanks for explaining! D119342 moves slightly closer to removing SizeLevel from the pass pipeline setup.

In D119342#3308172, @fhahn wrote:

Should we do this when the optsize function attribute is set, not just when the minsize function attribute is set?

I don't think so. Disabling rotation is a heavy hammer, as it will disable a lot of other loop optimizations implicitly due to most transforms requiring rotated form.

After some more testing on a larger amount of code (many small programs, together over 1MB in binary size), LoopRotate indeed seems to be the culprit. I'm now looking into a patch to LoopRotate to respect the optsize function attribute.

Apparently there is also another patch that tries to do something very similar: D81223.

I would be very interested in this patch, because for me to use ThinLTO without size regressions I need to set the optimization size level in the linker (PassManagerBuilder.SizeLevel etc).
This patch seems mostly correct to me, except for the function attributes. These function attributes (optsize, minsize) should IMHO be set in the frontend, not in the linker.

• small update to the load8.ll test to make it more readable

• use getIORegSREG() instead of hardcoding I/O address 0x3f.

Hmm, I realize that this is not the best approach. I will make a new patch. You are correct that some instructions had the wrong register class operand.

Looks good to me!

Looks good to me! A few suggestions below (which aren't necessary, just ideas).

I found out that avr-gcc does not support reusing the same ldi instruction for multiple elpm instructions, so if we can do that, we're better than avr-gcc :)
https://godbolt.org/z/Yzhaj7e54

This looks pretty good. Thanks for the improvements!

Looks good to me!

I'm not sure this is the correct location for these checks. You're essentially checking whether the compilation looks like a C/C++ compilation or an assembly compilation based on the flags and the file name. However, the Clang driver already does something like this: it converts the command line arguments and files into a list of jobs to perform. This is done in Driver::BuildActions, Driver::BuildJobs, Clang::ConstructJob, and other places.
I think a better place to do this check is in Clang::ConstructJob. There is already something similar here:

Looks good to me.

I've looked through the code again. It looks correct and (in past testing) it fixed a bug I had, so I'm going to accept this.

(just so that this doesn't appear in my review queue anymore)

Can you please add some .mir tests? As an example, I have created one for ASRW7Rd and ASRW8Rd:

• rebase (try 2, previous one was the wrong patch)

• rebased

In D116551#3221666, @benshi001 wrote:

Just one suggestion, run llvm/utils/update_llc_test_checks.py against the test case llvm/test/CodeGen/AVR/interrupts.ll

• replaced CHECK: with CHECK-NEXT:

I actually made several patches to the AVR backend to make it machine verifier clean, so it was certainly not incorrectly disabled. Here are the patches: D96969, D97127, D97172, D97131, D97159. This did in fact find a few actual bugs, so I'm very happy with the work on the machine verifier!