[PATCH] D107133: [AVR] emit `MCSA_Global` references to `__do_global_ctors` and `__do_global_dtors`

[Ben Shi via Phabricator via llvm-commits]

benshi001 added inline comments.


================
Comment at: llvm/lib/Target/AVR/AVRAsmPrinter.cpp:64
   const MCRegisterInfo &MRI;
+  bool EmittedStructorSymbolAttrs = false;
 };
----------------
benshi001 wrote:
> mhjacobson wrote:
> > benshi001 wrote:
> > > It would not be good to use such a flag. But would be better to put your code into MCTargetDesc/AVRTargetStreamer.cpp where `__do_copy_data` is emitted, and make a check like
> > > 
> > > ```
> > > if (current module has constructor)
> > >   emit do_global_ctors
> > > if (current module has destructor)
> > >   emit do_global_dtors
> > > ```
> > I'm not sure how to know, from `AVRTargetStreamer`, whether there are constructors/destructors.
> > 
> > If we don't want to keep `EmittedStructorSymbolAttrs`, then I suppose an alternative would be for `AVRAsmPrinter::emitXXStructor()` *always* to emit the global symbols.  It won't hurt anything for them to be emitted more than once -- it's just theoretically inefficient.
> > 
> > Can you explain more about your concern with `EmittedStructorSymbolAttrs`?  Is it bad to keep state inside `AVRAsmPrinter`?
> Yes. I do concern keep any state inside `AVRAsmPrinter` ? How about Dylan's opinion?
I really do not like using a flag to keep the emitted state. How about only directly emit them in MCTargetDesc/AVRTargetStreamer.cpp, and add a FIXME, as for `__do_copy_data` ?

```
void AVRTargetStreamer::finish() {
  MCStreamer &OS = getStreamer();
  MCContext &Context = OS.getContext();

  MCSymbol *DoCopyData = Context.getOrCreateSymbol("__do_copy_data");
  MCSymbol *DoClearBss = Context.getOrCreateSymbol("__do_clear_bss");

  // FIXME: We can disable __do_copy_data if there are no static RAM variables.

  OS.emitRawComment(" Declaring this symbol tells the CRT that it should");
  OS.emitRawComment("copy all variables from program memory to RAM on startup");
  OS.emitSymbolAttribute(DoCopyData, MCSA_Global);

  OS.emitRawComment(" Declaring this symbol tells the CRT that it should");
  OS.emitRawComment("clear the zeroed data section on startup");
  OS.emitSymbolAttribute(DoClearBss, MCSA_Global);
}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107133/new/

https://reviews.llvm.org/D107133