Re: [C/C++ PATCH] Call make_decl_rtl in {,c_}determine_visibility if DECL_RTL is set and visibility changed (PR target/39175)

[Mark Mitchell <mark at codesourcery dot com>]

Jakub Jelinek wrote:

>> Yes, I guess the patch is OK.  You're right that we clearly can't do
>> anything radical at this point.  But:
> 
> Is that OK also for the C FE side?

Yes.  (If Joseph objects, though, his objection trumps my approval.)

> Looking at notice_global_symbol, we could avoid calling make_decl_rtl
> in many cases just by checking MEM_P (DECL_RTL (decl)) later, as done in
> the following untested patch (when weak_global_object_name is already non-NULL
> and decl is weak, one only or -fpic, there is no need to call
> make_decl_rtl).  But that doesn't help the original testcase, as foo is the
> first symbol in the CU, for which notice_global_symbol will still call
> make_decl_rtl.

This is good analysis.  Yes, I think your change to notice_global_symbol
is incremental goodness, and worth doing.

However, what should really be done is to get rid of the use of DECL_RTL
altogether in that function in favor of something that doesn't depend on
RTL.  I would think that DECL_ASSEMBLER_NAME would work just about as
well as targetm.strip_name_encoding -- we're just trying to get a
globally unique name.  That would also reduce the target-specificity
here, which would be nice, since this (getting a unique name for the
module) is a logically front-endish kind of thing.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713