llvm dev - Jul 2007 - [LLVMdev] Couple of changes (2005 and other toolchain related)

Hi,

I upgraded the Visual Studio SLN file to work with 2005 and had to make some
changes.

The first two have to do with the fact that the debug implementation of
2005's
STL does all sort of validation, that's why they didn’t show up on 2003.

I'm not set up for patch submission yet, but if somebody has time to review
these changes that'd be greatly appreciated.

Meanwhile I'll try to get setup with patch, diff and a cvs drop. I have some
other things I'd like to contribute (a new AsmWriter, and proper
cross-compilation support from X86 to PPC).

Thanks,

Jaap Suter - http://jaapsuter.com


* PredicateSimplifier.cpp, line 236, add: friend bool operator<(unsigned to,
const Edge &edge) { return to < edge.To; }
* PredicateSimplifier.cpp, line 677, add:	friend bool operator<(const Value
*value, const ScopedRange &range) { return value < range.V; }

Lacking these operators, it generates something like the following:

	c:\program files\microsoft visual studio 8\vc\include\xutility(267) : error
C2678: binary '<' : no operator found which takes a left-hand operand
of type
'const unsigned int' (or there is no acceptable conversion)
	c:\libraries\llvm-2.0\lib\transforms\scalar\predicatesimplifier.cpp(677): could
be 'bool `anonymous-namespace'::operator <(const llvm::Value *,const
`anonymous-namespace'::ValueRanges::ScopedRange &)' [found using
argument-
dependent lookup]
	        while trying to match the argument list '(const unsigned int,
`anonymous-namespace'::InequalityGraph::Edge)'
	       
c:\libraries\llvm-2.0\lib\transforms\scalar\predicatesimplifier.cpp(295) : see
reference to function template instantiation '_FwdIt
std::lower_bound<`anonymous-namespace'::InequalityGraph::Node::iterator,unsigned
int>(_FwdIt,_FwdIt,const _Ty &)' being compiled
	        with
	        [
	            _FwdIt=`anonymous-namespace'::InequalityGraph::Node::iterator,
	            _Ty=unsigned int
	        ]

This has to do with the fact that the STL debug validator checks to make sure
that if !(lhs < rhs) then (rhs < lhs || rhs == lhs), so the '<'
operator needs
to be non-ambiguous.

* BranchFolding.cpp, line 927, replace with:

	std::size_t dist = std::distance(PI, MBB->pred_begin());
      ReplaceUsesOfBlockWith(*PI, MBB, CurTBB, TII);
	PI = MBB->pred_begin();
	std::advance(PI, dist);

I'm not sure if that is the correct fix, but what is currently there is
definitely not correct. ReplaceUsesOfBlockWith can invalidate iterators (because
internally it adds or removes items from containers) so we need to reset PI if
that happens.

* TypeSymbolTable.h, MachineModuleInfo.h and Annotation.cpp all use a const
sd::string as the key for a std::hash_table or std:map. Keys for containers are
assumed to be immutable and not all STL implementations like the superfluous
consts. This wasn't a 2005 problem, but related to a different STL
implementation we use as well. I'm not sure what the official C++ standard
says,
but removing the const doesn't hurt here.

* PPCISelLowering.cpp has a function that just asserts(false) right now. To
avoid a warning, I added a "return SDOperand();"

* RSProfling.cpp, SimplifyLibCalls.cpp, InlineFunction.cpp, changed use of NULL
to (char*)NULL in calls to getOrInsertFunction. This fixes a missing sentinel
warning (http://www.linuxonly.nl/docs/sentinel/)

Hola Jaap,

I'm curious which version of the source are you working with?  It sounds
like you and I were working on the same problem yesterday, but I didn't
see those particular compiler errors.  (I saw a couple of other ones for
which I submitted a patch).

I did see errors like the ones you saw with the CVS LLVM 2.0 sources a
while back, namely the missing < operator and the debug STL in VStudio
2k5, but those were fixed in the CVS trunk and were carried over to the
SVN tree. 

I haven't sync'd to the SVN trunk in the last day and half.  I'll do
so
now and check.

Thanks,
Chuck.



-----Original Message-----
From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu]
On Behalf Of Jaap Suter
Sent: Thursday, July 26, 2007 5:02 PM
To: LLVM Developers Mailing List
Subject: [LLVMdev] Couple of changes (2005 and other toolchain related)

Hi,

I upgraded the Visual Studio SLN file to work with 2005 and had to make
some
changes.

The first two have to do with the fact that the debug implementation of
2005's
STL does all sort of validation, that's why they didn't show up on 2003.

I'm not set up for patch submission yet, but if somebody has time to
review
these changes that'd be greatly appreciated.

Meanwhile I'll try to get setup with patch, diff and a cvs drop. I have
some
other things I'd like to contribute (a new AsmWriter, and proper
cross-compilation support from X86 to PPC).

Thanks,

Jaap Suter - http://jaapsuter.com


* PredicateSimplifier.cpp, line 236, add: friend bool operator<(unsigned
to,
const Edge &edge) { return to < edge.To; }
* PredicateSimplifier.cpp, line 677, add:	friend bool
operator<(const Value
*value, const ScopedRange &range) { return value < range.V; }

Lacking these operators, it generates something like the following:

	c:\program files\microsoft visual studio
8\vc\include\xutility(267) : error
C2678: binary '<' : no operator found which takes a left-hand operand
of
type
'const unsigned int' (or there is no acceptable conversion)
	
c:\libraries\llvm-2.0\lib\transforms\scalar\predicatesimplifier.cpp(677)
: could
be 'bool `anonymous-namespace'::operator <(const llvm::Value *,const
`anonymous-namespace'::ValueRanges::ScopedRange &)' [found using
argument-
dependent lookup]
	        while trying to match the argument list '(const unsigned
int,
`anonymous-namespace'::InequalityGraph::Edge)'
	       
c:\libraries\llvm-2.0\lib\transforms\scalar\predicatesimplifier.cpp(295)
: see
reference to function template instantiation '_FwdIt
std::lower_bound<`anonymous-namespace'::InequalityGraph::Node::iterator,
unsigned 
int>(_FwdIt,_FwdIt,const _Ty &)' being compiled
	        with
	        [
	
_FwdIt=`anonymous-namespace'::InequalityGraph::Node::iterator,
	            _Ty=unsigned int
	        ]

This has to do with the fact that the STL debug validator checks to make
sure
that if !(lhs < rhs) then (rhs < lhs || rhs == lhs), so the '<'
operator
needs
to be non-ambiguous.

* BranchFolding.cpp, line 927, replace with:

	std::size_t dist = std::distance(PI, MBB->pred_begin());
      ReplaceUsesOfBlockWith(*PI, MBB, CurTBB, TII);
	PI = MBB->pred_begin();
	std::advance(PI, dist);

I'm not sure if that is the correct fix, but what is currently there is
definitely not correct. ReplaceUsesOfBlockWith can invalidate iterators
(because
internally it adds or removes items from containers) so we need to reset
PI if
that happens.

* TypeSymbolTable.h, MachineModuleInfo.h and Annotation.cpp all use a
const
sd::string as the key for a std::hash_table or std:map. Keys for
containers are
assumed to be immutable and not all STL implementations like the
superfluous
consts. This wasn't a 2005 problem, but related to a different STL
implementation we use as well. I'm not sure what the official C++
standard says,
but removing the const doesn't hurt here.

* PPCISelLowering.cpp has a function that just asserts(false) right now.
To
avoid a warning, I added a "return SDOperand();"

* RSProfling.cpp, SimplifyLibCalls.cpp, InlineFunction.cpp, changed use
of NULL
to (char*)NULL in calls to getOrInsertFunction. This fixes a missing
sentinel
warning (http://www.linuxonly.nl/docs/sentinel/)


_______________________________________________
LLVM Developers mailing list
LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

> I'm curious which version of the source are you working with?
I was working on 2.0...    :-)
> I haven't sync'd to the SVN trunk in the last day and half. 
I'll do so
> now and check.
Don't bother then, I should have synced to CVS and made sure those fixes 
didn't already exist.

After I posted my previous email, I found two more problems. One is a 
function with that doesn't return a value (it calls abort(), but 2005 gives 
a warning), the other is another iterator invalidation problem. I'll have a 
look at the CVS and see if those were addressed already as well.

Thanks,

Jaap Suter