[LLVMdev] Built LLVM 1.8 on VC8, invalid iterator issue/fix, some questions

[srhilber at ncsu.edu]

Hello,

I've managed to get LLVM 1.8 to build properly with MSVC8 with some
effort. Most of the changes necessary were very minor C++ usage issues;
for instance, VC8 can't deal with prototyping something that's really a
class as 'struct Foo;' or vice versa (it creates issues with matching
function signatures during linking.) Additionally, I had to do quite a few
updates to the included VC7.1 project file after doing an upgrade to a VC8
project (the project file appears to be somewhat out of date.)

However, there appears to be some issue with using an invalid iterator in
the X86 backend that appears when I build with VC8. I've managed to fix it
by duplicating the std::vector in question before iterating through it,
but I'm too new to the code to be able to say whether it's the "right"
fix. The offending bit of generated code is this area from
X86GenDAGISel.inc, line 84+:

	// ReplaceHandles - Replace all the handles with the real target
	// specific nodes.
	void ReplaceHandles() {
	  for (std::map<SDOperand, SDOperand>::iterator I = ReplaceMap.begin(),
	        E = ReplaceMap.end(); I != E; ++I) {
	    SDOperand From = I->first;
	    SDOperand To   = I->second;
	    for (SDNode::use_iterator UI = From.Val->use_begin(), E =
From.Val->use_end(); UI != E; ++UI) {
	      SDNode *Use = *UI;
	      std::vector<SDOperand> Ops;
	      for (unsigned i = 0, e = Use->getNumOperands(); i != e; ++i){
	        SDOperand O = Use->getOperand(i);
	        if (O.Val == From.Val)
	          Ops.push_back(To);
	        else
	          Ops.push_back(O);
	      }
	      SDOperand U = SDOperand(Use, 0);
	      CurDAG->UpdateNodeOperands(U, Ops); // *** This invalidates the UI
iterator ***
	    }
	  }
	}

This causes lli to crash on the ++UI in the for loop (assert failure in
debug mode, hard crash nearby in release mode) if I try to run bytecode
generated from this LLVM code:

	%blah = global int 5

	implementation

	int %main()
	{
		%foo = load int* %blah

		; Causes crash if I do this instead of
		; a constant like 'ret int 0'
		ret int %foo
	}

My hack around the situation was to copy From.Val into a temporary
std::vector<SDNode*>, by modifying the code that generates that code
(TableGen/DAGISelEmitter.cpp), but that's got a performance penalty:

	// ...
	std::vector<SDNode*> tempnodes(From.Val->use_begin(), From.Val->use_end());
    for (SDNode::use_iterator UI = tempnodes.begin(); UI !=
tempnodes.end(); UI++) {
    // ...

I could preallocate the temporary vector to a certain size to help a
little, but ideally there's no copying of vectors at all.

NOTE: I haven't tried running the testsuite on the fixed binaries to see
if the build is 100% working yet, but at least that issue appears
temporarily solved.

Anyway, I've got a few questions:
1) First thing, is VC8 support desired at the moment? If so I can try to
put together a patch, although I'd likely have to install VS7.1 to edit
the project file while maintaining compatibility with 7.1.
2) Could someone who knows more about the context of the invalid iterator
issue tell me what the 'right' fix would be? Is this just something
g++/libstdc++ happens to let pass silently and it really needs changing,
or is VC8 screwing it up somehow? If it does needs changing, what's the
proper way of doing it?
3) Should I be posting this to bugzilla instead? ;)

Thanks,
Scott Hilbert