[MSP430] Initial fixes for built-ins on MSP430

[atrosinenko]

Here are mostly guessed fixes to compiler-rt that make it compile for MSP430 target. These are definitely WIP at the time of writing.

It can be compiled with something like

llvm_build=/path/to/llvm/build-directory
sysroot=/path/to/msp430gcc

cflags="-target msp430 -mmcu=msp430g2553 --gcc-toolchain=$sysroot -I$sysroot/include -L$sysroot/include"

cmake "$@" ../compiler-rt -G Ninja \
    -DCOMPILER_RT_DEFAULT_TARGET_TRIPLE="msp430-none-elf" \
    -DCOMPILER_RT_EMULATOR="$sysroot/bin/msp430-elf-run" \
    -DCOMPILER_RT_INCLUDE_TESTS=ON \
    -DCOMPILER_RT_BUILD_BUILTINS=ON \
    -DCMAKE_C_COMPILER=$llvm_build/bin/clang \
    -DCMAKE_CXX_COMPILER=$llvm_build/bin/clang++ \
    -DCMAKE_AR=$llvm_build/bin/llvm-ar \
    -DCMAKE_NM=$llvm_build/bin/llvm-nm \
    -DCMAKE_RANLIB=$llvm_build/bin/llvm-ranlib \
    -DLLVM_CONFIG_PATH=$llvm_build/bin/llvm-config \
    -DCMAKE_C_FLAGS="$cflags" \
    -DCMAKE_CXX_FLAGS="$cflags" \
    -DCMAKE_ASM_FLAGS="$cflags" \
    -DCOMPILER_RT_BAREMETAL_BUILD=ON

Prior to #20 the __ELF__ macro have to be set as well.

[atrosinenko]

I have managed to run most of compiler-rt unit tests via msp430-elf-run simulator (shipped with gdb, see updated PR description) when I forgot dropping some -lgcc from linker command line. :) Without -lgcc, there are obviously lots of unresolved references to __mspabi_XYZ. According to MSP430TargetLowering, most of them are just renamed libgcc symbols, so what is the preferred way to map these names?

[asl]

Create an alias? Similar to ARM EABI functions?

[atrosinenko]

Yes, I expected something similar to other targets. As I understand, sometimes it is defined just like the following:

llvm-project/compiler-rt/lib/builtins/muldi3.c

Lines 38 to 51 in 40ae542

	COMPILER_RT_ABI di_int __muldi3(di_int a, di_int b) {
	dwords x;
	x.all = a;
	dwords y;
	y.all = b;
	dwords r;
	r.all = __muldsi3(x.s.low, y.s.low);
	r.s.high += x.s.high * y.s.low + x.s.low * y.s.high;
	return r.all;
	}
	#if defined(__ARM_EABI__)
	COMPILER_RT_ALIAS(__muldi3, __aeabi_lmul)
	#endif

at the end of common .c file (not taking into account native asm implementations naturally residing under 'builtins/arm/'). I would prefer to describe all these aliases in a single .c or .S file under builtins/msp430 instead, but this is probably against the policy of separate files for separate functions. On the other hand, spreading such tiny aliases across lots of files may be quite hard to review and may be not very maintainable.

Ideally, I expect some autogenerated file that would be aligned with MSP430TargetLowering by construction (still, I'm not sure it is possible in general case).

[asl]

Just put into separate files :)

[atrosinenko]

Just to not perform massive rewriting afterwards :) Do I get it right, separate files builtins/msp430/__mspabi_XYZ.S, each containing a standard copyright header and a couple of macroses aliasing it to existing generic implementation for now? (Except for some msp-specific routines, of course)

[asl]

Ok. In msp430 directory

[atrosinenko]

The problem with aliasing via msp430/*.S was that I cannot alias a symbol from another object file (this can be done, but with linker script, not just another mspabi-aliases.o...).

On the other hand, it seems not too handy to spread aliases for every new target across .c files with corresponding generic implementations. What if just add an ALIAS<defined function name> macro "invocation" for every libcall? Then only a single "generic" header needs an adjustment for every new target, while all target-specific aliases can reside in a single header, not cluttering every generic .c file.

[asl]

Sorry, I do not follow.

[atrosinenko]

While googling how to emit some symbol as an alias to another one, I have found some suggestions on using .set to_symbol, from_symbol. But, as I understood, there exist some generic issues for referencing a symbol not defined in the same object file (relocations don't work for that cases, at least generically, they say...) and wasting 4*renamed_symbol_count bytes for this (br original_symbol) may be not very good for such low-memory target.

[atrosinenko]

So, there exist other possibilities: either the most trivial one - use assignments via a linker script or alias in the correct translation unit. Linker script may require some non-trivial building. On the other hand, aliasing in the generic C-sources are already used for ARM target. My proposed approach just make it possible to localize changes for new target support in a couple of files instead of spreading them over at most O(libcall count) files.

PS:

1. My patch still have to touch every required libcall source file
2. In the latest commit of this PR, there is only a single file manually patched as an example
3. ARM aliases may probably be refactored that way if appropriate

[asl]

We cannot use linker script in general. It may be custom or not used at all. So, may I suggest us to go simple for now and mimic what ARM EABI is doing?

[atrosinenko]

Current check-builtins status:

Failing Tests (36):
  Builtins-msp430-linux :: absvdi2_test.c
  Builtins-msp430-linux :: absvsi2_test.c
  Builtins-msp430-linux :: addtf3_test.c
  Builtins-msp430-linux :: ashldi3_test.c
  Builtins-msp430-linux :: ashrdi3_test.c
  Builtins-msp430-linux :: clzsi2_test.c
  Builtins-msp430-linux :: compiler_rt_logb_test.c
  Builtins-msp430-linux :: compiler_rt_logbf_test.c
  Builtins-msp430-linux :: ctzsi2_test.c
  Builtins-msp430-linux :: divdc3_test.c
  Builtins-msp430-linux :: divdf3_test.c
  Builtins-msp430-linux :: divsc3_test.c
  Builtins-msp430-linux :: divsf3_test.c
  Builtins-msp430-linux :: divtc3_test.c
  Builtins-msp430-linux :: fixdfdi_test.c
  Builtins-msp430-linux :: fixsfdi_test.c
  Builtins-msp430-linux :: fixunsdfdi_test.c
  Builtins-msp430-linux :: fixunssfdi_test.c
  Builtins-msp430-linux :: floatdidf_test.c
  Builtins-msp430-linux :: floatdisf_test.c
  Builtins-msp430-linux :: floatundidf_test.c
  Builtins-msp430-linux :: floatundisf_test.c
  Builtins-msp430-linux :: lshrdi3_test.c
  Builtins-msp430-linux :: muldc3_test.c
  Builtins-msp430-linux :: mulodi4_test.c
  Builtins-msp430-linux :: mulsc3_test.c
  Builtins-msp430-linux :: mulvdi3_test.c
  Builtins-msp430-linux :: paritydi2_test.c
  Builtins-msp430-linux :: paritysi2_test.c
  Builtins-msp430-linux :: popcountdi2_test.c
  Builtins-msp430-linux :: popcountsi2_test.c
  Builtins-msp430-linux :: powidf2_test.c
  Builtins-msp430-linux :: powisf2_test.c
  Builtins-msp430-linux :: subtf3_test.c
  Builtins-msp430-linux :: truncdfsf2_test.c
  Builtins-msp430-linux :: udivmoddi4_test.c


Testing Time: 2.04s
  Unsupported Tests  : 95
  Expected Passes    : 71
  Expected Failures  :  1
  Unexpected Failures: 36

[asl]

I would ignore all FP tests for now and try to understand the integer ones.

[atrosinenko]

Not surprisingly, my draft implementations for msp430-specific shift LibCalls were wrong :)

After they were fixed:

  Unsupported Tests  : 95
  Expected Passes    : 82
  Expected Failures  :  1
  Unexpected Failures: 31

Specifically, the following tests were fixed:

  Builtins-msp430-linux :: ashldi3_test.c
  Builtins-msp430-linux :: ashrdi3_test.c
  Builtins-msp430-linux :: clzsi2_test.c
  Builtins-msp430-linux :: ctzsi2_test.c
  Builtins-msp430-linux :: lshrdi3_test.c

and 6 other were added (for these 6 shift LibCalls).

[atrosinenko]

Shouldn't this be a part of COMPILER_RT_ABI?

According to MSP430 EABI, "6.3 Special Register Conventions for Helper Functions", there is only a small subset of __mspabi_* functions that operate on two 64-bit arguments - they use this optimization. So this is probably correct but slightly misleading: CallingConv::MSP430_BUILTIN is that "modified" convention and not the one used for every LibCall. It would probably be better to leave it as-is and add an explanation.

[asl]

What will break if we'd use this CC for all builtins?

[atrosinenko]

Right now, for example, I have adjusted MSP430CallingConv.td by adding

  CCIfCC<"CallingConv::MSP430_BUILTIN", CCPromoteToType<i64>>,

Still it is just guessed in many ways (but seems to ultimately work...), so there may exist more correct way to express this. I agree, using MSP430_BUILTIN for, well, all MSP430 builtins would make sense :)

But I'm not sure this can be trivially achieved without specially marking LibCalls that need such an adjustment. For example, part 6.3 lists "The following functions take 2 64-bit values". Among others, there is __mspabi_srlll function. But even in MSP430 EABI Table 10, it is declared as uint64 __mspabi_srlll(uint64 x, int16 n);, so one cannot just match on LibCall with two i64 arguments. This still may be achievable but it would require further investigations.

[atrosinenko]

  Unsupported Tests  : 95
  Expected Passes    : 94
  Expected Failures  :  1
  Unexpected Failures: 22

[asl]

Can we have it msp430 specific for now?

[asl]

Can we have it msp430 specific?

[asl]

Can we have *hi builtins move to msp430 folder, please?

[atrosinenko]

Can we have *hi builtins move to msp430 folder, please?

Done. I have left their names unchanged (that is, traditional compiler-rt name aliased to _mspabi*). As I understand, now they are "staging" but someday they may probably become part of 16-bit target support (and then be moved to common directory).

[atrosinenko]

Failing Tests (11):
  Builtins-msp430-linux :: addtf3_test.c
  Builtins-msp430-linux :: comparedf2_test.c
  Builtins-msp430-linux :: compiler_rt_logbf_test.c
  Builtins-msp430-linux :: divdc3_test.c
  Builtins-msp430-linux :: divsc3_test.c
  Builtins-msp430-linux :: divtc3_test.c
  Builtins-msp430-linux :: muldc3_test.c
  Builtins-msp430-linux :: mulsc3_test.c
  Builtins-msp430-linux :: mulvdi3_test.c
  Builtins-msp430-linux :: subtf3_test.c
  Builtins-msp430-linux :: udivmoddi4_test.c

Testing Time: 9.08s
  Unsupported Tests  :  95
  Expected Passes    : 105
  Expected Failures  :   1
  Unexpected Failures:  11

[asl]

I would concentrate on udivmoddi4_test.c

[atrosinenko]

I would concentrate on udivmoddi4_test.c

Chances are, it will "just work" when I ultimately manage to link it. The problem is enabling optimization refuses to "just work", so I'm investigating it.

/old_ssd/ast/soft/msp430gcc/lib/gcc/msp430-elf/8.3.1/../../../../bin/msp430-elf-ld: /old_ssd/ast/msp430-projects/llvm-project/build-compiler-rt-msp430/test/builtins/Unit/MSP430LinuxConfig/Output/udivmoddi4_test.c.tmp section `.data' will not fit in region `RAM'
/old_ssd/ast/soft/msp430gcc/lib/gcc/msp430-elf/8.3.1/../../../../bin/msp430-elf-ld: section __interrupt_vector_31 loaded at [000000000000fffe,000000000000ffff] overlaps section .data loaded at [00000000000049a8,00000000000a58d5]
/old_ssd/ast/soft/msp430gcc/lib/gcc/msp430-elf/8.3.1/../../../../bin/msp430-elf-ld: region `RAM' overflowed by 616676 bytes
clang-11: error: msp430-elf-ld command failed with exit code 1 (use -v to see invocation)

[asl]

Why is it so large? What is so special in it as compared to other tests?