This is the 32-bit division operation for AVR (both unsigned and signed). The function works, but could probably be optimized further somewhat. It will certainly be a lot more efficient than a C version, but most importantly it matches the ABI expected by avr-gcc and LLVM (unlike the compiler-rt C version).
Does it make sense to do something like this instead:
#if defined(__AVR_TINY__) #error "avrtiny is not yet supported" #endif
This would ensure that someone building for the environment is not greeted with a missing function.
Please use DEFINE_COMPILER_RT_FUNCTION from assembly.h.
Please use SYMBOL_NAME from assembly.h.
Similar to the other file.
I don't think so, because then you can't use compiler-rt anymore for these chips (while currently you can). That would be a regression. A future patch can always add support for these little chips.
For context, avrtiny chips often have just 1kB of flash for program storage (with the largest chips having 4kB). That's 512 instructions. Code will likely be so trivial and/or optimized that you're probably not going to need a 32-bit division anyway.
I would like to agree... but unfortunately there is an issue. The macro uses an assembly separator string. For most architectures, this is %% but for avr-gcc this is $. For Clang this is currently %% but that's a bug, we should match avr-gcc here (fixed in D138535).
Are you okay with that?
I guess this doesn't make much sense without DEFINE_COMPILERRT_FUNCTION? (See above).
Right, I realize that it would mean that you cannot build the library for that target, but that seems better than finding that there are missing symbols there. If we want to continue to support that chip as a build platform, we should just filter the source file out in the build system.
Why could we not accomodate in assembly.h? We can do a check for __clang__ and even change it based on the clang version. That way we can continue to support them backwards and forwards.
There are missing symbols right now: that's what this patch is trying to fix. This patch as-is is a strict improvement over what we already have. What this patch does is add __divmodsi4 and __udivmodsi4 to non-avrtiny chips, and leave the avrtiny chips as they already are.
Well, actually, it's a little bit more complicated. There exists the generic divmod implementations but they have a different ABI and so don't work on any AVR chips. So technically this patch will change from miscompiling code on avrtiny to causing a linker error, which IMHO is still an improvement. In summary:
So yeah this is not complete, but is an improvement and I hope to add __divmodsi4 etc support for avrtiny in a future patch.
I tried various separators (%%, %, $) but they don't work under LLVM 14. For example, for %% (which in theory would have worked), I get the following error message:
[...]/llvm-project.main/compiler-rt/lib/builtins/avr/divmodhi4.S:29:2: error: unexpected token at start of statement %% .globl __divmodhi4 %% .type __divmodhi4,@function %% __divmodhi4:
If you know of a way to still support LLVM 14 then please let me know. Otherwise, I think the only reasonable way forward is to not use DEFINE_COMPILERRT_FUNCTION for now.