[llvm-commits] [llvm-gcc-4.2] r56903 - in /llvm-gcc-4.2/trunk/gcc: llvm-backend.cpp llvm-convert.cpp

[Duncan Sands]

Hi Bill,

> -#include "c-common.h"

yay!  Removal of a C-specific file from the language independent llvm-backend.cpp!

> +#include "c-common.h"

Argh!  Addition of a C-specific file to the language independent llvm-convert.cpp!

> +  if (!flag_no_builtin && !builtin_function_disabled_p("memcpy")) {

Woah, this call means total death for Fortran and Ada!

> +      (IntPtr == Type::Int32Ty) ? Intrinsic::memcpy_i32 : Intrinsic::memcpy_i64;
...
> +        Intrinsic::memmove_i32 : Intrinsic::memmove_i64;
...
> +      (IntPtr == Type::Int32Ty) ? Intrinsic::memset_i32 : Intrinsic::memset_i64;

If I understand right, here you are outputting a call to "memcpy" rather than
"llvm.memcpy" if no-builtins is turned on.  Why?  Does this win you anything?
I thought no-builtins meant: 'if in my program I call "memcpy" leave it alone
and don't turn it into something else'.  But that's not the case here.
Here a struct copy is being turned into a call to llvm.memcpy.  That will
be turned by the code generators into either an inline code sequence
or a call to "memcpy".  Is there anything wrong with that?  No user call to
memcpy is being turned into an inline code sequence.  For what it's worth
gcc itself happily synthesizes calls to builtin_memcpy (eg: in
gimplify_modify_expr_to_memcpy, a similar situation).

As for the remaining changes, I don't see why they are needed either.  The
code you changed only turns things marked as builtins by gcc into llvm
intrinsics.  If builtins are turned off in gcc then presumably user calls
to these functions will not be marked as gcc builtin calls in the first place,
so already won't be turned into llvm builtins.  I didn't test this though.

In short, while I understand that you are trying to fix something, I'm not sure
what it is and how your changes fix it!  Instead you seemed to have fixed things
that weren't broken, and broken non-C languages...

Ciao,

Duncan.