Re: Native compiler - passing command line options to C compiler

[Arthur Miller]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Arthur Miller <arthur.miller@live.com>
>> Date: Sun, 29 Aug 2021 23:47:56 +0200
>> 
>> after the few mails the other day, I wasn't really sure if Andrea is going to
>> implement it and when. I thought it was rather a tedious manual labour and 
>> maybe
>> not so important, so I took me a liberty to implement this myself in my own, 
>> so
>> called, personal copy of Eamcs sources.
>
> Thanks.
>
>> I am not sure if I have done it correctly though, I appreciate if Andrea have
>> time to take a look; I have just mainly copied your code for backend 
>> options. It
>> seems to work for me, with a minor remark: When I pass a valid option, 
>> "native",
>> in place where it should go, I get an invalid option error. Gcc even lists 
>> it in
>> the error message as a valid option. Another option "skylake" works just 
>> fine. This seems to vary between flags. I am not sure if this is some 
>> encoding
>> error from Emacs to libgccjit, or if it is some bug in libgccjit, or is it 
>> just
>> my brain having dumps.
>
> I guess -march=native is something handled by GCC itself, and here we
> don't have it?  If you want to be sure, ask this question on the GCC
> list, or report as a bug to their Bugzilla.
>
>> +break your code. Use at own risk.
>                   ^^
> Two spaces between sentences.

Ah, why did I forgott that one :). 

>> +DEFUN ("comp-native-compiler-options-effective-p",
>> +       Fcomp_native_compiler_options_effective_p,
>> +       Scomp_native_compiler_options_effective_p,
>> +       0, 0, 0,
>> +       doc: /* Return t if `comp-native-compiler-options' is effective.  */)
>> +  (void)
>> +{
>> +#if defined (LIBGCCJIT_HAVE_gcc_jit_context_add_command_line_option)  \
>> +  || defined (WINDOWSNT)  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>> +  if (gcc_jit_context_add_command_line_option)
>> +    return Qt;
>> +#endif
>
> The emphasized part doesn't look right: we did that elsewhere because
> the options we pass there work around bugs that happen also in
> versions that don't report libgccjit version.  But here this is not
> needed, and the version check isn't present anyway.  So the WINDOWSNT
> special handling should be removed, I think.

Ok. I am not familiar with the details of how and why, so I just left it. I can
remove it.

>> +static void
>> +add_compiler_options (void)
>> +{
>> +  Lisp_Object options = Fsymbol_value (Qnative_comp_compiler_options);
>> +
>> +#if defined (LIBGCCJIT_HAVE_gcc_jit_context_add_command_line_option) \
>> +  || defined (WINDOWSNT)
>> +  load_gccjit_if_necessary (true);
>> +  if (!NILP (Fcomp_native_compiler_options_effective_p ()))
>
> Likewise here.  And since Fcomp_native_compiler_options_effective_p
> already does this test, why did you need to have another test outside
> it?

That was just copy pasta. I have noticed that and reflected over that, but I
didn't want to poke too much into stuff. 

>> +  /* Captured `comp-native-driver-options' because file-local.  */
>> +#if defined (LIBGCCJIT_HAVE_gcc_jit_context_add_command_line_option) \
>> +  || defined (WINDOWSNT)
>> +  options = comp.compiler_options;
>> +  if (!NILP (Fcomp_native_compiler_options_effective_p ()))
>> +    FOR_EACH_TAIL (options)
>> +      gcc_jit_context_add_command_line_option (comp.ctxt,
>> +                                               /* FIXME: Need to encode
>> +                                                  this, but how? either
>> +                                                  ENCODE_FILE or
>> +                                                  ENCODE_SYSTEM.  */
>> +                                               SSDATA (XCAR (options)));
>> +#endif
>
> Likewise here.

Yes, I'll fix Windows case.  If you would like me to test it on Windows it will 
take
time. Maybe tomorrow or day after. I can send in patch when I tested to save
everyones time.

Thanks for the help and clarifications!