diff mbox

C++ PATCH for c++/51827 (mangling error with PCH and LTO)

Message ID CAFiYyc2QKfR=J5V-VF7HSJ_mnVNARtw+d_i33YaHG5dYehFNig@mail.gmail.com
State New
Headers show

Commit Message

Richard Biener Jan. 17, 2012, 9:35 a.m. UTC
On Mon, Jan 16, 2012 at 10:29 PM, Jason Merrill <jason@redhat.com> wrote:
> When outputting PCH/LTO, the compiler tries to generate mangled names for
> all decls before discarding language-specific data.  But that doesn't make
> sense for templates, and leads to conflicts in this case. Fixed by refusing
> to mangle templates.
>
> Tested x86_64-pc-linux-gnu, applying to trunk.

Hmm.  Using -flto when generating a precompiled header doesn't make sense.
We probably should drop -flto silently or complain here.  Thus,

any objection to that?

Thanks,
Richard.

Comments

Gabriel Dos Reis Jan. 17, 2012, 1:21 p.m. UTC | #1
On Tue, Jan 17, 2012 at 3:35 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Mon, Jan 16, 2012 at 10:29 PM, Jason Merrill <jason@redhat.com> wrote:
>> When outputting PCH/LTO, the compiler tries to generate mangled names for
>> all decls before discarding language-specific data.  But that doesn't make
>> sense for templates, and leads to conflicts in this case. Fixed by refusing
>> to mangle templates.
>>
>> Tested x86_64-pc-linux-gnu, applying to trunk.
>
> Hmm.  Using -flto when generating a precompiled header doesn't make sense.
> We probably should drop -flto silently or complain here.  Thus,
>
> Index: c-family/c-opts.c
> ===================================================================
> --- c-family/c-opts.c   (revision 183205)
> +++ c-family/c-opts.c   (working copy)
> @@ -1058,6 +1058,13 @@ c_common_post_options (const char **pfil
>       && flag_preprocess_only && !flag_no_line_commands)
>     pp_dir_change (parse_in, get_src_pwd ());
>
> +  /* Disable LTO output when outputting a precompiled header.  */
> +  if (pch_file && flag_lto)
> +    {
> +      flag_lto = 0;
> +      flag_generate_lto = 0;
> +    }
> +
>   return flag_preprocess_only;
>  }
>
> any objection to that?

I think we should issue a diagnostic about the combination of these
two flags.  Silent ignorance would lead to false user expectations
and supposed "bug" reports, if not confusion.

-- Gaby
Diego Novillo Jan. 17, 2012, 1:23 p.m. UTC | #2
On Tue, Jan 17, 2012 at 04:35, Richard Guenther
<richard.guenther@gmail.com> wrote:

> +  /* Disable LTO output when outputting a precompiled header.  */
> +  if (pch_file && flag_lto)
> +    {
> +      flag_lto = 0;
> +      flag_generate_lto = 0;
> +    }
> +

Emit a warning when you do this?


Diego.
Richard Biener Jan. 17, 2012, 2:43 p.m. UTC | #3
On Tue, Jan 17, 2012 at 2:23 PM, Diego Novillo <dnovillo@google.com> wrote:
> On Tue, Jan 17, 2012 at 04:35, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>
>> +  /* Disable LTO output when outputting a precompiled header.  */
>> +  if (pch_file && flag_lto)
>> +    {
>> +      flag_lto = 0;
>> +      flag_generate_lto = 0;
>> +    }
>> +
>
> Emit a warning when you do this?

Well, I'd say peoples Makefiles may not strip -flto properly here (you need to
pass optimization level for example), so I'm not sure that would be good QOI.

> I think we should issue a diagnostic about the combination of these
> two flags.  Silent ignorance would lead to false user expectations
> and supposed "bug" reports, if not confusion.

I'm not sure what "expectation" there would be?

OTOH the bug is fixed by Jason, so we can just do nothing.

Richard.
Gabriel Dos Reis Jan. 17, 2012, 2:50 p.m. UTC | #4
On Tue, Jan 17, 2012 at 8:43 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:

>> I think we should issue a diagnostic about the combination of these
>> two flags.  Silent ignorance would lead to false user expectations
>> and supposed "bug" reports, if not confusion.
>
> I'm not sure what "expectation" there would be?

that PCH works with LTO :-)

(especially when PCH means precompiled header :-)

-- Gaby
Richard Biener Jan. 17, 2012, 2:51 p.m. UTC | #5
On Tue, Jan 17, 2012 at 3:50 PM, Gabriel Dos Reis
<gdr@integrable-solutions.net> wrote:
> On Tue, Jan 17, 2012 at 8:43 AM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>
>>> I think we should issue a diagnostic about the combination of these
>>> two flags.  Silent ignorance would lead to false user expectations
>>> and supposed "bug" reports, if not confusion.
>>
>> I'm not sure what "expectation" there would be?
>
> that PCH works with LTO :-)
>
> (especially when PCH means precompiled header :-)

Sure it works.  PCH is just a stage where LTO is not active yet.

Richard.

> -- Gaby
Jason Merrill Jan. 17, 2012, 4:22 p.m. UTC | #6
On 01/17/2012 09:51 AM, Richard Guenther wrote:
> Sure it works.  PCH is just a stage where LTO is not active yet.

That makes sense to me.

Jason
Richard Biener Jan. 18, 2012, 1:10 p.m. UTC | #7
On Tue, Jan 17, 2012 at 5:22 PM, Jason Merrill <jason@redhat.com> wrote:
> On 01/17/2012 09:51 AM, Richard Guenther wrote:
>>
>> Sure it works.  PCH is just a stage where LTO is not active yet.
>
>
> That makes sense to me.

Applied then.

Richard.

> Jason
>
diff mbox

Patch

Index: c-family/c-opts.c
===================================================================
--- c-family/c-opts.c   (revision 183205)
+++ c-family/c-opts.c   (working copy)
@@ -1058,6 +1058,13 @@  c_common_post_options (const char **pfil
       && flag_preprocess_only && !flag_no_line_commands)
     pp_dir_change (parse_in, get_src_pwd ());

+  /* Disable LTO output when outputting a precompiled header.  */
+  if (pch_file && flag_lto)
+    {
+      flag_lto = 0;
+      flag_generate_lto = 0;
+    }
+
   return flag_preprocess_only;
 }