[PATCH] D81461: GlobalISel: Fix double printing new instructions in legalizer

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 9 06:00:00 PDT 2020


arsenm created this revision.
arsenm added reviewers: aditya_nandakumar, aemerson, paquette, dsanders, gargaroff, foad, kerbowa, nhaehnle.
Herald added subscribers: hiraditya, rovka, wdng.
Herald added a project: LLVM.
arsenm added a comment.

Actually I think this isn't the main source of double printing. The same observer seems to get called twice:

First through this:
#0  llvm::GISelObserverWrapper::createdInstr (this=0x7fffffffb658, MI=...)
#1  0x0000000003b9ca51 in llvm::GISelObserverWrapper::MF_HandleInsertion (this=0x7fffffffb658, MI=...)
#2  0x00000000055f703e in llvm::MachineFunction::handleInsertion (this=0x792f510, MI=...)

And then later this:
#0  (anonymous namespace)::LegalizerWorkListManager::createdInstr (this=0x7fffffffb6e0, MI=...)
#1  0x0000000003b9cb88 in llvm::GISelObserverWrapper::createdInstr (this=0x7fffffffb658, MI=...)
#2  0x0000000003b9ca51 in llvm::GISelObserverWrapper::MF_HandleInsertion (this=0x7fffffffb658, MI=...)


arsenm added a comment.

It seems both the MachineFunction and MachineIRBuilder have insertion observers; this seems redundant. I think the machine function observers should be removed?


New instructions were getting printed both in createdInstr, and in the
final printNewInstrs, so it made it look like the same instructions
were created twice. This overall made reading the debug output
harder. Stop printing the initial construction and only print new
instructions in the summary at the end. This avoids printing the less
useful case where instructions are sometimes initially created with no
operands.

      

I'm not sure this is the correct instance to remove; now the visible
ordering is different. Now you will typically see the one erased
instruction message before all the new instructions in order. I think
this is the more logical view of typical legalization changes,
although it's mechanically backwards from the normal
insert-new-erase-old pattern.


https://reviews.llvm.org/D81461

Files:
  llvm/lib/CodeGen/GlobalISel/Legalizer.cpp


Index: llvm/lib/CodeGen/GlobalISel/Legalizer.cpp
===================================================================
--- llvm/lib/CodeGen/GlobalISel/Legalizer.cpp
+++ llvm/lib/CodeGen/GlobalISel/Legalizer.cpp
@@ -133,7 +133,6 @@
   }
 
   void createdInstr(MachineInstr &MI) override {
-    LLVM_DEBUG(dbgs() << ".. .. New MI: " << MI);
     LLVM_DEBUG(NewMIs.push_back(&MI));
     createdOrChangedInstr(MI);
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D81461.269500.patch
Type: text/x-patch
Size: 414 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200609/cc06a218/attachment.bin>


More information about the llvm-commits mailing list