[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;
 };
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107133