[PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

[Philipp Krones via Phabricator via llvm-commits]

flip1995 marked an inline comment as done.
flip1995 added inline comments.


================
Comment at: clang/tools/driver/cc1as_main.cpp:407
 
-  MOFI->initMCObjectFileInfo(Ctx, PIC);
+  // FIXME: This is not pretty. MCContext has a ptr to MCObjectFileInfo and
+  // MCObjectFileInfo needs a MCContext reference in order to initialize itself.
----------------
MaskRay wrote:
> Can we fix the FIXME now?
Not quite. The construction is still in the order `MCContext` -> `MOFI` -> "continue construction of `MCContext` by setting the `MOFI`".  The order just changed from "dummy `MOFI`" -> `MCContext` -> "init dummy `MOFI`".


================
Comment at: llvm/include/llvm/MC/MCObjectFileInfo.h:255
 
+  virtual unsigned getTextSectionAlignment() const { return 4; }
   MCSection *getTextSection() const { return TextSection; }
----------------
MaskRay wrote:
> This should be moved to D102052
Will do.


================
Comment at: llvm/lib/DWARFLinker/DWARFStreamer.cpp:57
 
-  MOFI.reset(new MCObjectFileInfo);
-  MC.reset(
-      new MCContext(TheTriple, MAI.get(), MRI.get(), MOFI.get(), MSTI.get()));
-  MOFI->initMCObjectFileInfo(*MC, /*PIC=*/false);
+  MC.reset(new MCContext(TheTriple, MAI.get(), MRI.get(), /*MOFI=*/nullptr,
+                         MSTI.get()));
----------------
MaskRay wrote:
> flip1995 wrote:
> > MaskRay wrote:
> > > The argument is almost always `/*MOFI=*/nullptr`. Doesn't this regress a bit?
> > I agree. I think the best thing to do here would be to just remove this argument?
> I think so. Since MOFI is guaranteed to be constructed after MCContext now. 
Will do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101921