Respin the builtin PR

[dkm]

Add 1 extra on top of previous PR from Arthur that
adds attribute support.

Must be squashed with the original to get a working change.

[CohenArthur]

ah, while doing a quick review I noticed that this commit also includes some of the new intrinsics I was adding at the time @dkm. I have no idea if these are correct or not, and you can probably drop them if you'd like to avoid some headache

[dkm]

ah, while doing a quick review I noticed that this commit also includes some of the new intrinsics I was adding at the time @dkm. I have no idea if these are correct or not, and you can probably drop them if you'd like to avoid some headache

I'll try to add some test for said intrinsics (I didn't notice them while adjusting the patch, but I guess I'll find them easily?). Would be better to have them in now, while I'm at it :)

[dkm]

ah, while doing a quick review I noticed that this commit also includes some of the new intrinsics I was adding at the time @dkm. I have no idea if these are correct or not, and you can probably drop them if you'd like to avoid some headache

I'll try to add some test for said intrinsics (I didn't notice them while adjusting the patch, but I guess I'll find them easily?). Would be better to have them in now, while I'm at it :)

@CohenArthur can you elaborate a bit on which intrinsics you were trying to add?

[dkm]

@CohenArthur should be in a good enough state. Haven't touched too much at your code, but fixed some shortcoming (mainly the attributes that were dropped when including the .def file).
If/when it's approved, I'll squashed both and mark mysel as Co-authored-by so that the bootstrap is happy (hopefully).

[CohenArthur]

Looks great @dkm! Thank you for taking the time :) I'll run a bootstrap build on it if you want before we merge it

[dkm]

Looks great @dkm! Thank you for taking the time :) I'll run a bootstrap build on it if you want before we merge it

I've already tested the bootstrap build on x86_64-linux, but feel free to test again, we don't want to introduce any bootstrap issues :)

[dkm]

Let me first squash everything!

[dkm]

Hmmm, there are issues on gcc 4.8 and MacOS.

[dkm]

Strangely, the compiler doesn't ICE locally, will try with a gcc 4.8.

 215   │ crab1: internal compiler error: Segmentation fault
 216   │ 0x12e528f crash_signal
 217   │     ../../gcc/toplev.cc:314
 218   │ 0x7f730443650f ???
 219   │     ./signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0
 220   │ 0xccd827 Rust::Compile::BuiltinsContext::define_function_type(Rust::Compile::BuiltinsContext::Type, Rust::Compile::BuiltinsContext::Type, bool, unsigned long, ...)
 221   │     ../../gcc/rust/backend/rust-builtins.cc:65
 222   │ 0xcd28bb Rust::Compile::BuiltinsContext::define_builtin_types()
 223   │     ../../gcc/builtin-types.def:967
 224   │ 0xcd3c78 Rust::Compile::BuiltinsContext::setup()
 225   │     ../../gcc/rust/backend/rust-builtins.cc:358
 226   │ 0xcd3d3b Rust::Compile::BuiltinsContext::get()
 227   │     ../../gcc/rust/backend/rust-builtins.cc:30

[dkm]

That's fun, valgrind output loads of error with a gccrs built using gcc-4.8 as the base compiler, but it's clean with a gccrs built using a more recent base compiler.

[CohenArthur]

That's fun, valgrind output loads of error with a gccrs built using gcc-4.8 as the base compiler, but it's clean with a gccrs built using a more recent base compiler.

yuuuuuuumm :) C++almost11 strikes again!!

[dkm]

That's fun, valgrind output loads of error with a gccrs built using gcc-4.8 as the base compiler, but it's clean with a gccrs built using a more recent base compiler.

yuuuuuuumm :) C++almost11 strikes again!!

But it seems it's not related to C++11.
The code crashed on:

      auto arg_idx = va_arg (list, size_t);
      auto arg_type = builtin_types[arg_idx];

with arg_idx being ridiculously large. The original D code has the same sequence but it pulls int not size_t from ..., and indeed, using int works® and matches the symptoms (size_t being 8 and int being 4 bytes, on lp64).

But the "real" argument being passed is an enum Type, and using this in the va_arg call gives a nice warning (thank you compiler):

../../gcc/rust/backend/rust-builtins.cc: In member function ‘void Rust::Compile::BuiltinsContext::define_function_type(Rust::Compile::BuiltinsContext::Type, Rust::Compile::Bu
iltinsContext::Type, bool, size_t, ...)’:                                                                                                                                     
../../gcc/rust/backend/rust-builtins.cc:64:36: warning: ‘Rust::Compile::BuiltinsContext::Type’ is promoted to ‘int’ when passed through ‘...’ [enabled by default]            
       auto arg_idx = va_arg (list, Type);                                                                                                                                    
                                    ^                                                                                                                                         
../../gcc/rust/backend/rust-builtins.cc:64:36: note: (so you should pass ‘int’ not ‘Rust::Compile::BuiltinsContext::Type’ to ‘va_arg’)                                        
../../gcc/rust/backend/rust-builtins.cc:64:36: note: if this code is reached, the program will abort           

See SO post about that: https://stackoverflow.com/questions/24580503/error-when-pass-enum-in-a-function-with-variable-arguments

[dkm]

MacOS 😭

[dkm]

MacOS 😭

ok, easy fix...