User Details
- User Since
- Apr 25 2018, 9:12 AM (212 w, 5 d)
Wed, May 11
This file is also linked to libclang.a.
Fri, May 6
Looks good to me. It might be possible to optimize these functions a bit more but that can happen at a later time.
Thu, May 5
@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.
Wed, May 4
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
Mon, May 2
Apr 20 2022
ping
Apr 12 2022
Not a real review but I found this which looks like a bug.
Apr 7 2022
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.
Mar 26 2022
Mar 25 2022
@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.
Mar 24 2022
Mar 23 2022
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.
Mar 22 2022
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.
Mar 12 2022
Mar 1 2022
Ping?
Feb 10 2022
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.
Feb 9 2022
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.
Feb 8 2022
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.
Feb 2 2022
Jan 23 2022
- small update to the load8.ll test to make it more readable
Jan 22 2022
- 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.
Jan 20 2022
Jan 19 2022
Looks good to me!
Looks good to me! A few suggestions below (which aren't necessary, just ideas).
Jan 18 2022
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:
Jan 17 2022
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:
Jan 16 2022
Jan 6 2022
- rebase (try 2, previous one was the wrong patch)
- rebased
- 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!