DAGISelEmitter not looking up instruction namespace accurately.

[Quuxplusone]

Bugzilla Link	PR1001
Status	RESOLVED FIXED
Importance	P normal
Reported by	Nikhil Patil (patil.nikhil@gmail.com)
Reported on	2006-11-11 00:28:54 -0800
Last modified on	2010-02-22 12:53:43 -0800
Version	trunk
Hardware	PC Linux
CC	llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

I was trying to write my own simple LLVM backend for my own simple ISA when I
ran into the following error message:

MCGenDAGISel.inc: In member function
'llvm::SDNode*<unnamed>::MCDAGToDAGISel::SelectCode(llvm::SDOperand)':
MCGenDAGISel.inc:463: error: 'INSTRUCTION_LIST_END' is not a member of
'llvm::TargetInstrInfo'

My target description had only one instruction NOP. The problem disappears when
I change NOP to ADD. In fact anything *alphabetically* before INLINEASM is
okay. (INLINEASM & PHINODE are in namespace TargetInstrInfo.) I have traced
down the problem to utils/TableGen/DAGISelEmitter.cpp:3390, where we only look
at the first instruction in the list of all instructions. So if there happen to
be no instructions alphabetically before INLINEASM, we get screwed.

Please let me know if you need any more information :)

[Quuxplusone]

This looks like a relatively easy bug to fix, but a hard one for me to test
without an example.  Can you
propose a patch please?

Thanks,

-Chris

[Quuxplusone]

There's two ways. (I think I'd prefer (a).)

(a) Check for TargetInstrInfo explicitly

--- DAGISelEmitter.cpp  8 Nov 2006 23:01:03 -0000       1.281
+++ DAGISelEmitter.cpp  14 Nov 2006 03:11:13 -0000
@@ -3387,7 +3387,13 @@
 }

 void DAGISelEmitter::EmitInstructionSelector(std::ostream &OS) {
-  std::string InstNS = Target.inst_begin()->second.Namespace;
+  std::string InstNS;
+  CodeGenTarget::inst_iterator i;
+  for (i = Target.inst_begin(); i != Target.inst_end(); ++i) {
+    InstNS = i->second.Namespace;
+    if (InstNS != "TargetInstrInfo")
+       break;
+  }
   if (!InstNS.empty()) InstNS += "::";

   // Group the patterns by their top-level opcodes.

-------------------------------------------------------------------

(b) Check for INLINEASM & PHI explicitly:

diff -u -r1.281 DAGISelEmitter.cpp
--- DAGISelEmitter.cpp  8 Nov 2006 23:01:03 -0000       1.281
+++ DAGISelEmitter.cpp  14 Nov 2006 03:16:36 -0000
@@ -3387,7 +3387,15 @@
 }

 void DAGISelEmitter::EmitInstructionSelector(std::ostream &OS) {
-  std::string InstNS = Target.inst_begin()->second.Namespace;
+  std::string InstNS;
+  CodeGenTarget::inst_iterator i;
+  for (i = Target.inst_begin(); i != Target.inst_end(); ++i) {
+    std::string name = i->second.TheDef->getName();
+    if (name != "INLINEASM" && name != "PHI") {
+       InstNS = i->second.Namespace;
+       break;
+    }
+  }
   if (!InstNS.empty()) InstNS += "::";

   // Group the patterns by their top-level opcodes.
-----------------------------------------------------------------------

Thanks,

[Quuxplusone]

Option #1 sounds good to me!  Thanks, I applied your patch:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20061120/040138.html

Thanks,

-Chris