Here are the two problem areas:
RegisterInfoEmitter.cpp
// Emit the subregister + index mapping function based on the
information
// calculated above.
OS << "unsigned " << ClassName
<< "::getSubReg(unsigned RegNo, unsigned Index) const {\n"
<< " switch (RegNo) {\n"
<< " default: abort(); break;\n";
...
OS << " };\n";
OS << " return 0; // Visual Studio 2005 does not respect the
no-return semantics of abort\n";
OS << "}\n\n";
Need this because otherwise the emitted function will fail to compile in
VStudio.
PredicateSimplifier.cpp:
The other problem is subtle. In the Edge class (for example) this
function will fail to compile in debug.
iterator find(unsigned n, ETNode *Subtree) {
iterator E = end();
for (iterator I = std::lower_bound(begin(), E, n);
I != E && I->To == n; ++I) {
if (Subtree->DominatedBy(I->Subtree))
return I;
}
return E;
}
The debug paths through VStudio's STL through the std::lower_bound
function get to this template:
template<class _Ty1, class _Ty2> inline
bool __CLRCALL_OR_CDECL _Debug_lt(const _Ty1& _Left, const _Ty2&
_Right,
const wchar_t *_Where, unsigned int _Line)
{ // test if _Left < _Right and operator< is strict weak
ordering
if (!(_Left < _Right))
return (false);
else if (_Right < _Left)
_DEBUG_ERROR2("invalid operator<", _Where, _Line);
return (true);
}
The critical feature is that the A<B (A an Edge and B an unsigned)
operator also ends up doing B<A in order to check that the comparator
isn't wonky. And since the B<A function wasn't written in
PredicateSimplifier.cpp, I needed to add some stuff like this:
// this one's already there
bool operator<(unsigned to) const {
return To < to;
}
// these two together get me the unsigned < Edge operation
bool operator>(unsigned to) const {
return To > to;
}
friend bool operator<(unsigned to, const Edge& edge) {
return edge.operator>(to);
}
There is a similar set of additions for ScopedRange.
Adding the alternate comparitors and the return won't make things less
portable, but it does crudify the code somewhat in order to get around
the limitations of the compiler. Having a bunch of #ifdefs for VStudio
probably is crudier still, however. :-)
What do you think?
Thanks,
Chuck.
-----Original Message-----
From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu]
On Behalf Of Chris Lattner
Sent: Thursday, May 31, 2007 4:00 PM
To: LLVM Developers Mailing List
Subject: Re: [LLVMdev] Advice on a VStudio specific patch
On Thu, 31 May 2007, Chuck Rose III wrote:> Our project is cross platform and on Windows we use VStudio 2005.
> VStudio presents a couple of issues related around it's STL
> implementation and also it's non-respect for the no-return semantic of
> abort().
Ok. We want the source to be portable, so it's goodness to get these
fixes into the main tree.
> I've fixed it locally, but I'd like to send a patch so I don't
have to
> do this every time I update from the source repository. So.... if I'm
> fixing something for a specific compiler, do you think I should just
do> so for all compilers or should I put the differences in a #ifdef check
> for VStudio?
Can you send one example of what you're thinking of? We prefer to keep
the main code #ifdef free, moving compiler-specific code to
include/llvm/Support/Compiler.h. Depending on what you mean, this may
or
may not make sense though :)
-Chris
--
http://nondot.org/sabre/
http://llvm.org/
_______________________________________________
LLVM Developers mailing list
LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
On Thu, 31 May 2007, Chuck Rose III wrote:> Here are the two problem areas: > RegisterInfoEmitter.cpp > // Emit the subregister + index mapping function based on the > information > // calculated above. > OS << "unsigned " << ClassName > << "::getSubReg(unsigned RegNo, unsigned Index) const {\n" > << " switch (RegNo) {\n" > << " default: abort(); break;\n"; > ... > OS << " };\n"; > OS << " return 0; // Visual Studio 2005 does not respect the > no-return semantics of abort\n"; > OS << "}\n\n"; > > Need this because otherwise the emitted function will fail to compile in > VStudio.Ok. For this, I suggest just removing the break after the abort. visual studio will just think it falls through into the next case, which is harmless.> operator also ends up doing B<A in order to check that the comparator > isn't wonky. And since the B<A function wasn't written in > PredicateSimplifier.cpp, I needed to add some stuff like this: > > // these two together get me the unsigned < Edge operation > bool operator>(unsigned to) const { > return To > to; > } > There is a similar set of additions for ScopedRange.Okay, those sound easy and simple to add unconditionally.> Adding the alternate comparitors and the return won't make things less > portable, but it does crudify the code somewhat in order to get around > the limitations of the compiler. Having a bunch of #ifdefs for VStudio > probably is crudier still, however. :-)Right, sounds good. Please add them unconditionally, so no #ifdef's required :) -Chris> -----Original Message----- > From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] > On Behalf Of Chris Lattner > Sent: Thursday, May 31, 2007 4:00 PM > To: LLVM Developers Mailing List > Subject: Re: [LLVMdev] Advice on a VStudio specific patch > > On Thu, 31 May 2007, Chuck Rose III wrote: >> Our project is cross platform and on Windows we use VStudio 2005. >> VStudio presents a couple of issues related around it's STL >> implementation and also it's non-respect for the no-return semantic of >> abort(). > > Ok. We want the source to be portable, so it's goodness to get these > fixes into the main tree. > >> I've fixed it locally, but I'd like to send a patch so I don't have to >> do this every time I update from the source repository. So.... if I'm >> fixing something for a specific compiler, do you think I should just > do >> so for all compilers or should I put the differences in a #ifdef check >> for VStudio? > > Can you send one example of what you're thinking of? We prefer to keep > the main code #ifdef free, moving compiler-specific code to > include/llvm/Support/Compiler.h. Depending on what you mean, this may > or > may not make sense though :) > > -Chris > >-Chris -- http://nondot.org/sabre/ http://llvm.org/
On 31/05/07, Chuck Rose III <cfr at adobe.com> wrote:> Here are the two problem areas: > > RegisterInfoEmitter.cpp > > // Emit the subregister + index mapping function based on the > information > // calculated above. > OS << "unsigned " << ClassName > << "::getSubReg(unsigned RegNo, unsigned Index) const {\n" > << " switch (RegNo) {\n" > << " default: abort(); break;\n"; > ... > OS << " };\n"; > OS << " return 0; // Visual Studio 2005 does not respect the > no-return semantics of abort\n"; > OS << "}\n\n"; > > Need this because otherwise the emitted function will fail to compile in > VStudio. >This might be more subtle than it seems. I'm assuming that since that actually fails, you have warnings-as-errors on. In compilers with good flow analysis that do have noreturn abort, the extra return should trigger an "unreachable code" warning, which will also prevent compilation with treat warnings as errors on... I know boost has a BOOST_UNREACHABLE_RETURN(x) macro for that situation; Perhaps LLVM should have something similar? ~ Scott McMurray
Nope, it generates an error, not a warning-as-error. (LLVM generates a whole host of warnings when run through the VStudio STL implementation). I misspoke earlier about VStudio ignoring noreturn. I investigated further and found that the implementation of abort in VStudio 2k5 is not tagged noreturn, hence the error. Thanks, Chuck. -----Original Message----- From: llvmdev-bounces at cs.uiuc.edu on behalf of me22 Sent: Thu 5/31/2007 7:11 PM To: LLVM Developers Mailing List Subject: Re: [LLVMdev] Advice on a VStudio specific patch On 31/05/07, Chuck Rose III <cfr at adobe.com> wrote:> Here are the two problem areas: > > RegisterInfoEmitter.cpp > > // Emit the subregister + index mapping function based on the > information > // calculated above. > OS << "unsigned " << ClassName > << "::getSubReg(unsigned RegNo, unsigned Index) const {\n" > << " switch (RegNo) {\n" > << " default: abort(); break;\n"; > ... > OS << " };\n"; > OS << " return 0; // Visual Studio 2005 does not respect the > no-return semantics of abort\n"; > OS << "}\n\n"; > > Need this because otherwise the emitted function will fail to compile in > VStudio. >This might be more subtle than it seems. I'm assuming that since that actually fails, you have warnings-as-errors on. In compilers with good flow analysis that do have noreturn abort, the extra return should trigger an "unreachable code" warning, which will also prevent compilation with treat warnings as errors on... I know boost has a BOOST_UNREACHABLE_RETURN(x) macro for that situation; Perhaps LLVM should have something similar? ~ Scott McMurray _______________________________________________ LLVM Developers mailing list LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
On Thu, 31 May 2007, me22 wrote:> I know boost has a BOOST_UNREACHABLE_RETURN(x) macro for that > situation; Perhaps LLVM should have something similar?This sounds like overkill to me :). LLVM has lots of other similar examples. -Chris -- http://nondot.org/sabre/ http://llvm.org/