@andreisfr awesome! Thanks a lot!

Also, there is an emulator here: https://github.com/espressif/qemu/wiki
I have successfully used it in the past. Noting it here as it can make backend review/development easier for those who don't have the hardware.

@andreisfr any update on this?

Yes, I was not aware of the special %A0/%B0 syntax, see my comment above.

In D97127#2602032, @benshi001 wrote:

In D97127#2600159, @aykevl wrote:

In D97127#2593329, @benshi001 wrote:

Do we need a test case to show the right AVR assembly ? Or at least shows that the points #1 and #2 in your comment are fixed.

I think that would imply using hardcoded registers in the test. Is that what you mean?

No. I meant, you have shown a case void atomicadd(_Atomic char *val) and corresponding assembly, and I would like to see the change of the generated assembly by your patch, and wonder if the change can be added as a test case.

Oh, now I understand. Apparently there is a %A0/%B0 syntax that I've never seen before in any documentation with which you can select parts of a register.
Yes, this looks good to me, although perhaps @dylanmckay should also take a look?

Oh, I missed this comment.

I don't really understand what you're doing here. For example:

• fixes so that shl i32 undef, undef doesn't crash

Re-upload (without changes) to try to get the buildbot working...

Ok, committed.

In D97127#2593329, @benshi001 wrote:

Do we need a test case to show the right AVR assembly ? Or at least shows that the points #1 and #2 in your comment are fixed.

• fix lint warning

• fix lint warnings

Nevermind, that won't work. This looks like the right way forward.

I think there is a better way to do this: by always using a frame pointer when a function needs a call frame. That would simplify the code a lot.

In D97131#2586533, @benshi001 wrote:

Why not commit it? I think this bug fix should be fixed ASAP.

Looks reasonable to me. But again, I would like this to be reviewed also by someone familiar with the internals of Clang (I'm not).

In D97172#2593337, @benshi001 wrote:

We need to fix the build failure before commit this patch.

I have a fix ready and will post a patch soon.

I have found that my fix isn't complete, it can result in broken code because the register allocator does not know that R31R30 is in use between ADJCALLSTACKDOWN and STDWSPQRr and might use it as the value operand for the STDWSPQRr instruction.

I should have explained this a while ago, sorry for neglecting this.

Thank you for the review!

• merge D97171

Let's merge this with D97159.

In D96677#2577490, @benshi001 wrote:

I am not sure such a specific pass is needed. Why __ashlsi3 is not called in other backends? Is there a config flag/option to prevent calling __ashlsi3 ?

I am not very familiar with Clang so I can't say much about it. Although I wonder whether a macro is the right way to implement this? Is there something similar in other targets? (GPUs tend to have lots of address spaces, you could take a look there).

• fix lint checks (hopefully)
• simplify pass a bit, with suggestions from @benshi001
• use update_test_checks.py for the test

• fix lint warnings

• remove misleading comment
• adjust to fix lint warnings

• match avr-gcc output (bst, ror, bld)
• fix issue with the BLD instruction

Oops this is not correct.

Oops I uploaded the wrong patch.

Thank you!
I have created a bug report to include this fix in the LLVM 12 release: https://bugs.llvm.org/show_bug.cgi?id=49176

@benshi001 can you commit it or should I do it for you?

Thank you for the quick patch! I have verified locally that this does indeed fix the bug (my AVR tests pass again), so this looks good to me.

I checked this, and it seems good to me although I would like someone else to also take a look.

This looks good to me, although I would like someone else to take a look as well. I can confirm that these changes fix an issue on AVR: this patch (together with D86547) make __floatsisf correct on AVR while it would previously do something incorrect.

I found at least one code sample that is miscompiled.

I've been testing the llvm main branch and I found that this change introduces a miscompilation. I have not yet investigated what exactly is going wrong.

@SjoerdMeijer thank you for the quick review!

Thanks for the review! I've updated the patch accordingly.

• added comment to shouldExpandAtomicCmpXchgInIR explaining Cortex-M exception
• added checks for armv8m architecture (which are the same as armv7m)

Yeah I was wondering the same thing when I saw the failure. Unfortunately such design is a bit outside of my LLVM knowledge. I would just like to use cc1as_main.cpp functionality outside of LLVM without needing to update my copy of cc1as with every LLVM update.

@echristo do you have an idea what's going on or how to fix this? I suspect I'm not including a required dependency or maybe I've put AssemblerInvocation in the wrong directory/library.
I'm not very familiar with CMake or C++ so I'm not sure how to best fix this.

Well that didn't quite work. I get errors like this:

Thanks! I have updated this patch to match LLVM main and committed it.

In D86418#2239289, @dylanmckay wrote:

Great patch

I made this patch to get more familiar with these inline expansions, in the hope that I can also do the other expansions inline (such as 32-bit shifts).

That would be nice!

I haven't verified this all but this looks reasonable to me, at least until a better way is figured out to store MCU specific information in the compiler.

See my comment D89047#2366709.
I think this optimization would be implemented better in SelectionDAG instead of using pseudo-instructions. That would also make it possible to optimize things like (short)x << 12.

The original AVR backend TD is also wrong, that it maps the bswap IR to AVR's swap instruction.

Since the IR bswap is byte level operation, which requires the minimal data type to be at least 16-bit.

But AVR's swap performs on half-byte level.