Add hexagon support

[androm3da]

No description provided.

[androm3da]

As I understand it this will add the target-specific builtins that are currently implemented in assembly in rust/src/llvm-project/compiler-rt to libcompiler-builtins.

@Amanieu or someone else please let me know if this is the right approach or if instead I need to port the assembly files over to inline assembly.

[Amanieu]

It's fine to use C or assembly implementations, as long as these are optional. Essentially, it should be possible to have a working target using only the intrinsics provided by the Rust code. The C/asm versions are only to provide optimized implementations.

[androm3da]

It's fine to use C or assembly implementations, as long as these are optional. Essentially, it should be possible to have a working target using only the intrinsics provided by the Rust code. The C/asm versions are only to provide optimized implementations.

Ok so compiler-builtins couldn't/shouldn't take this PR as-is, until the implementations are also provided in rust?

Is there a way to alias __hexagon_memcpy_likely_aligned_min32bytes_mult8bytes or __hexagon_sqrtf to instead use their C library equivalent?

[androm3da]

It's fine to use C or assembly implementations, as long as these are optional. Essentially, it should be possible to have a working target using only the intrinsics provided by the Rust code. The C/asm versions are only to provide optimized implementations.

Ok so compiler-builtins couldn't/shouldn't take this PR as-is, until the implementations are also provided in rust?

Is there a way to alias __hexagon_memcpy_likely_aligned_min32bytes_mult8bytes or __hexagon_sqrtf to instead use their C library equivalent?

I mean, they're presumably "optional" if we tell the compiler not to emit these calls at all. But perhaps they could also be optional if we did the mapping with some alias/flag for the linker? But if not that then I could probably write a rust implementation to explicitly forward each of these calls to the compiler-builtins target-independent implementation of memcpy / sqrtf in rust?

[androm3da]

Essentially, it should be possible to have a working target using only the intrinsics provided by the Rust code.

@Amanieu does that mean that rust functions that have inline asm would suffice here in lieu of the references to compiler-rt? Or should there still be a purely-rust implementation of each of these too?

[Amanieu]

Rust functions with inline assembly are fine. The main constraint is that we shouldn't need to depend on an external C compiler to build the target with -Z build-std.

[androm3da]

@Amanieu I think this is ready to review now.

Here are some items that I think are worth discussing:

allow(named_asm_labels)

• this lint doesn't work for hexagon assembly because register spans have colons in them so they're incorrectly detected as labels. I looked for labels in this code and the only labels I found are locals w/ leading '.' or leading '.L'.
• aliases: these aliases do define public symbols to alias with the start of the inline asm block, but hopefully that's acceptable. Note that we are using the .set directive and the lint would not have detected this. btw because of figure out some mechanism to alias symbols #70 I figured this is sorta unsolved and maybe acceptable workaround.

Symbol visibility

• the reference libclang_rt-builtins library has the arch specific symbols with default visibilty, and this commit has symbols with hidden visibility. All of these symbols should be available to other libraries when linking an executable and ... I suppose also when loading other shared objects (when being loaded with RTLD_GLOBAL?).
• I don't necessarily think it's a problem that they're hidden but it's a difference worth discussing.

formatting

• rustfmt doesn't touch much here. manual changes? preference?

[Amanieu]

This isn't what I had in mind. The problem with your current implementation is that the code is unmaintainable: it was copied from the asm in compiler-rt and mangled to work with asm!.

Either:

• Implement these using normal Rust code, calling our memcpy/soft-float implementations as needed. No assembly code should be needed unless there is a custom calling convention involved.
• Make cc a mandatory dependency for hexagon targets and just compile the .S files directly (after having copied them into compiler-builtins).

[androm3da]

This isn't what I had in mind. The problem with your current implementation is that the code is unmaintainable: it was copied from the asm in compiler-rt and mangled to work with asm!.

Well, shoot! I'm sorry I misunderstood. I don't think it's unmaintainable, though, is it? No more so than the other architecture-specific specializations already in compiler-builtins?

Either:

• Implement these using normal Rust code, calling our memcpy/soft-float implementations as needed. No assembly code should be needed unless there is a custom calling convention involved.

There's a reason we're overriding these operations: because the same Hexagon LLVM backend that clang uses generates suboptimal code for them. So we made the compiler emit calls to the optimized version instead. Why should rust programs perform worse than C ones for the same arithmetic operations?

• Make cc a mandatory dependency for hexagon targets and just compile the .S files directly (after having copied them into compiler-builtins).

IMO there's a benefit to having rust inline asm implementations here - it's just as you preferred - to eliminate a dependency on the C compiler. The reference implementations in compiler-rt/lib/builtins/hexagon/ are .S which require the C preprocessor.

Even though I think this PR gets the best behavior overall, I'm happy to do whatever it takes to suit compiler-builtins' needs. If maintenance is the primary concern - I'll commit to maintaining this work along with the tier 3 platform support for hexagon architecture triples. Or would committing to doing what is required for tier 2 suffice? And if that's not it, I'll just go ahead and implement the builtins in rust.

I can understand from your POV it might seem as if I am dumping a bunch of assembly for a special (VLIW) architecture, but I fully intend to support this code.

[Amanieu]

As a possible middle ground, would you consider placing your assembly code in .s files (note the lowercase, no C preprocessing) that can be used with global_asm? This avoids issues with symbol definitions not being allowed in inline assembly (even .L symbols are not allowed).

global_asm!(include_string!("foo.s"), options(raw));

[androm3da]

As a possible middle ground, would you consider placing your assembly code in .s files (note the lowercase, no C preprocessing) that can be used with global_asm?

Sure -- happy to do that.

[androm3da]

I also removed the hexagon-specific changes to build.rs because it looked to me like they are obsolete now that this content is in compiler-builtins. But please let me know if I'm mistaken there.

[androm3da]

Ok, sorry to drag this out @Amanieu - this recent push was to address build failures that showed up in release that were not present with debug build. There were macros with the same name defined among multiple assembly files and they were conflicting, as well as some conflicting label names. The specific organization of the assembly does seem to be slightly different among these, but the way it's documented indicates to me that these conflicts are the expected behavior. So I changed the assembly to comply.

This should be satisfactory now.