Bug 9761: EmitEndOfAsmFile is not the final emission point.

[arsenm]

Summary

MCStreamer::Finish() implementations add more code after the EmitEndOfAsmFile, which shouldn't happen, so move this after the Finish

[grosbach]

Totally reasonable. Please make sure to test very carefully on as many platforms as you have access to. I've been bitten lots of times by unintended consequences from cleanups like that. That is, finding all sorts of latent assumptions in the surrounding code.

[arsenm]

This breaks 6 tests. 2 of them are Darwin x86 (2010-12-02-MC-Set.ll and personality.ll) because of the placement of .subsections_via_symbols

The comment emitting it in X86AsmPrinter says

// Funny Darwin hack: This flag tells the linker that no global symbols
// contain code that falls through to other global symbols (e.g. the obvious
// implementation of multiple entry points).

So does the placement of this not actually matter? Is it OK to just move the expected location in the tests?

The other 4 are ARM and Mips object tests, which I don't understand yet.

[grosbach]

".subsections_via_symbols" should be the last thing in the file. It's mostly stylistic, but has been that way for a very long time, and I'd really prefer to keep it that way.

[arsenm]

.subsections_via_symbols isn't the last thing as it is now. This patch puts it there, breaking the 2 tests.

The broken object tests seem to be broken not from the content, but just the order things happen to occur in.

e.g. CodeGen/Thumb2/constant-islands.ll
The ARMMachObjectWriter is dependent on ARMAsmPrinter::EmitEndOfAsmFile calling MCObjectStreamer::EmitLabel to set the MCDataFragment on the nonlazy pointers (LLVM ERROR: symbol 'L__ZTV7RagDoll$non_lazy_ptr' can not be undefined in a subtraction expression), and I'm not sure where to move things around to fix this.

ARM and Mips ELF have similar problems. They add some flags in EmitEndOfAsmFile that are actually emitted in the OutStreamer Finish.

Should these move into the MC<ELF|MachO>Streamer::FinishImpls? It seems like target specific stuff doesn't belong there. Should there be a new function? The distinction between EmitEndOfAsmFile and MCStreamer::Finish is kind of confusing. Both "claim" to be the end of emission. The Emit* functions setting state that's used later feels wrong.

[grosbach]

Ah. OK. Those ARM failures aren't about subsections_via_symbols, really. This is other things that function does in addition to that.

This seems to be a reasonable explanation for why these functions are called in the order they are. Other than the name of the function being somewhat of a misnomer, what's the underlying motivation for the change? Perhaps there's a simpler solution.

[arsenm]

My problem is that AMDIL assumes the last thing in the program is "end", which is emitted in EmitEndOfAsmFile. When using debug info, the OutStreamer.Finish() dumps the line tables after it, which doesn't parse.

[arsenm]

I could probably fight with the thing that parses the debug output to get it to work, but I think it would make more sense if this function emitted the actual end.

[arsenm]

This is obsoleted by r194575 which just deletes the whole function.

[arsenm]

Whoops, wrong one

[arsenm]

No longer care about this target

lib/CodeGen/AsmPrinter/AsmPrinter.cpp

bool AsmPrinter::doFinalization(Module &M) {
 
   // If we don't have any trampolines, then we don't require stack memory
   // to be executable. Some targets have a directive to declare this.
   Function *InitTrampolineIntrinsic = M.getFunction("llvm.init.trampoline");
   if (!InitTrampolineIntrinsic || InitTrampolineIntrinsic->use_empty())
     if (const MCSection *S = MAI->getNonexecutableStackSection(OutContext))
       OutStreamer.SwitchSection(S);
 
+  OutStreamer.Finish();
+
   // Allow the target to emit any magic that it wants at the end of the file,
   // after everything else has gone out.
   EmitEndOfAsmFile(M);
 
-  delete Mang; Mang = 0;
-  MMI = 0;
-
-  OutStreamer.Finish();
   OutStreamer.reset();
 
+  delete Mang;
+  Mang = 0;
+  MMI = 0;
+
   return false;
 }
 
 void AsmPrinter::SetupMachineFunction(MachineFunction &MF) {
   this->MF = &MF;
   // Get the function symbol.
   CurrentFnSym = Mang->getSymbol(MF.getFunction());
   CurrentFnSymForSize = CurrentFnSym;