[PATCH] Generalize debug info / EH emission in AsmPrinter

Timur Iskhodzhanov timurrrr at google.com
Tue Nov 26 06:04:36 PST 2013



================
Comment at: lib/CodeGen/AsmPrinter/ARMException.cpp:57
@@ -59,3 +56,3 @@
 /// being emitted immediately after the function entry point.
-void ARMException::BeginFunction(const MachineFunction *MF) {
+void ARMException::beginFunction(const MachineFunction *MF) {
   getTargetStreamer().emitFnStart();
----------------
Eric Christopher wrote:
> Renames like this should probably happen separately.
OK, committed as r195763.

================
Comment at: lib/CodeGen/AsmPrinter/DwarfCFIException.cpp:87
@@ -86,3 +86,3 @@
 /// being emitted immediately after the function entry point.
-void DwarfCFIException::BeginFunction(const MachineFunction *MF) {
+void DwarfCFIException::beginFunction(const MachineFunction *MF) {
   shouldEmitMoves = shouldEmitPersonality = shouldEmitLSDA = false;
----------------
Eric Christopher wrote:
> Ditto with the renaming here.
Done

================
Comment at: lib/CodeGen/AsmPrinter/DIE.cpp:389
@@ -388,2 +388,3 @@
   // references have the same size as an address on the target system.
-  if (AP->getDwarfDebug()->getDwarfVersion() == 2)
+  unsigned DwarfVersion = AP->getDwarfVersion();
+  assert(DwarfVersion && "Expected Dwarf Debug info to be available");
----------------
Eric Christopher wrote:
> This is what I worry about from merging the EH and Debug Info classes. This assert should never be able to fire, and while "it's just an assert" theoretically we need that assert in a lot more places.
> 
> I guess what I'm saying is that it's not obvious the assert is necessary to me. How did you run into it in the first place?
This code is/was the only user of getDwarfDebug() method of AP.
I'm not at all familiar with DIEEntry, but looks like it just needs to know the DWARF version being used.

It looks like before it relied on DD to exist when getRefAddrSize executes.
Frankly, I'd put something like
  assert(AP->getDwarfDebug() != 0);
in this function regardless of my change.
It wasn't strictly necessary though as NULL deref would crash anyways.

That said, I believe merging EH and Debug Info has little to do with this.

I put the assert in just because I think calling getRefAddrSize() at the wrong time may silently lead to miracles.

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1832
@@ -1824,3 +1831,3 @@
   SmallPtrSet<const MDNode *, 16> ProcessedVars;
-  collectVariableInfo(MF, ProcessedVars);
+  collectVariableInfo(CurFn, ProcessedVars);
 
----------------
Eric Christopher wrote:
> Should be able to get rid of the MachineFunction * argument here yes?
Good point.
I also updated a couple of more functions taking MF* as their args.


http://llvm-reviews.chandlerc.com/D2222



More information about the llvm-commits mailing list