[PATCH] print .weak directives

The attached patch makes the AsmPrinter emit one .weak directive for
each weak global variable that is referenced at least once in a global
table. It uses a std::set to print only one directive.

Thoughts?

Best Regards,
Rafael

llvm.patch (2.88 KB)

I'm sorry, I must be missing something here:

In order to have weak linkage (I'm ignoring external weak for the moment) a global has to be defined in the LLVM code. Because it is defined in the LLVM code, it will have to be printed out by the target asm printer somewhere. When that happens, it will get a .weak directive emitted for it.

External weak globals are different: since they are not defined in the .ll file, it is not as simple to know whether or not to print them. There are two ways to handle them:

1a. Whenever we have a use of the global, print out a .weak directive.
1b. Whenever we have a use of the global, add the global to a set and
     print out .weak directives in doFinalization.
2. In doFinalization, scan all the globals and functions, emitting one
    .weak directive for anything with external weak linkage, whether it is
    used or not.

1a is suboptimal because extraneous .weak directives get emitted.
1b is what the asmprinter currently uses, but it misses uses from the
    constant pool.
2 is potentially bad because it will emit a .weak directive even if a
   global is not used. In practice, however, dead prototypes will have
   been removed before asmprinting anyway, so this potential slight
   inefficiency won't hurt anything.

You're proposing to add another set (to AsmPrinter) to keep track of extern weak globals. This could cause .weak directives to potentially be emitted twice (once in asmprinter once in armasmprinter, f.e.) and requires the cost of maintaining the sets.

I think it would be simpler and more efficient in the common case to do #2. What do you think? Am I missing something?

-Chris

I'm sorry, I must be missing something here:

In order to have weak linkage (I'm ignoring external weak for the moment)
a global has to be defined in the LLVM code. Because it is defined in the
LLVM code, it will have to be printed out by the target asm printer
somewhere. When that happens, it will get a .weak directive emitted for
it.

It should, but currently we don't print it:

I'm sorry, I must be missing something here:

In order to have weak linkage (I'm ignoring external weak for the moment)
a global has to be defined in the LLVM code. Because it is defined in the
LLVM code, it will have to be printed out by the target asm printer
somewhere. When that happens, it will get a .weak directive emitted for
it.

It should, but currently we don't print it:
----------------------------------
%a = weak global int 0
---------------------------------
       .comm a,4,4
--------------------------------
In fact, a bug in my patch is that with it we print both a .comm and a .weak :frowning:

Ah ok, the bug there is that a variable can't be .comm if it is weak.

External weak globals are different: since they are not defined in the .ll
file, it is not as simple to know whether or not to print them. There are
two ways to handle them:

I see. I was trying to handle them the same way since we were missing
.weak directives for both, but they must be handled differently.

Ok.

I see now that we have two problems:

1) print .weak directives for locally defined weak globals
2) print .weak directives for used external weak symbols

I will try to submit one patches for each problem. For the second one
I like 1b the best. Maybe we could use only one set (in AsmPrinter).
Both the AsmPrinter code and the back end specific code would add
symbols to it when they see a use of an external weak symbol and the
AsmPrinter would emit the directives in doFinalization.

Sounds good!

-Chris