Couple of changes (2005 and other toolchain related)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Couple of changes (2005 and other toolchain related)

Jaap Suter
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
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Couple of changes (2005 and other toolchain related)

Chuck Rose III
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: [hidden email] [mailto:[hidden email]]
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
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Couple of changes (2005 and other toolchain related)

Jaap Suter
> 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

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Couple of changes (2005 and other toolchain related)

Holger Schurig-2
> Don't bother then, I should have synced to CVS and made sure
> those fixes didn't already exist.

No, sync to SVN.
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Couple of changes (2005 and other toolchain related)

Chuck Rose III
In reply to this post by Jaap Suter
Hola Jaap,

Those problems you found with the 2.0 sources were fixed in the old CVS
tree right after 2.0.  Those fixes were carried over to the new SVN tree
(the one you should use now).  

There are a few VStudio errors that cropped up in the last month
concerning redefining variables in the interior block of a loop,
something VStudio doesn't like.  They should be addressed shortly.

Thanks,
Chuck.

-----Original Message-----
From: [hidden email] [mailto:[hidden email]]
On Behalf Of Jaap Suter
Sent: Thursday, July 26, 2007 8:33 PM
To: LLVM Developers Mailing List
Subject: Re: [LLVMdev] Couple of changes (2005 and other toolchain
related)

> 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

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev