[llvm-commits] CVS: llvm/tools/llvm-upgrade/ParserInternals.h UpgradeLexer.l UpgradeParser.y

Reid Spencer rspencer at reidspencer.com
Mon Jan 1 23:08:28 PST 2007


Hi Gordon,

On Tue, 2007-01-02 at 01:31 -0500, Gordon Henriksen wrote:
> Would it save much work to simply disambiguate all variable
> definitions which rely on type planes for uniqueness? 

That is, essentially, what llvm-upgrade is doing. 

> There would be no need to identify %t1 and %t2 as identical types, and
> the program would be immune to problems of this class. llvm-upgrade
> would still have to track the obsolete type planes in order to
> accurately update uses, though.

Yup, that's what it does.  Note however, that I've cited a fairly easy
case (that I'm working on now). However, consider something like this:

"struct.kc::hashtable_level" = type { bool,
"struct.__gnu_cxx::hash_set<kc::hashitem,kc::hash_hashitem,kc::eq_hashitem,std::allocator<kc::hashitem> >", "struct.std::map<std::basic_string<char, std::char_traits<char>, std::allocator<char> >,kc::impl_path*,std::less<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >,std::allocator<std::pair<const std::basic_string<char, std::char_traits<char>, std::allocator<char> >, kc::impl_path*> > >", "struct.std::map<std::basic_string<char, std::char_traits<char>, std::allocator<char> >,kc::impl_path*,std::less<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >,std::allocator<std::pair<const std::basic_string<char, std::char_traits<char>, std::allocator<char> >, kc::impl_path*> > >", "struct.std::map<std::basic_string<char, std::char_traits<char>, std::allocator<char> >,kc::impl_path*,std::less<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >,std::allocator<std::pair<const std::basic_string<char, std::char_traits<char>, std::allocator<char> >, kc::impl_path*> > >", "struct.std::map<std::basic_string<char, std::char_traits<char>, std::allocator<char> >,kc::impl_path*,std::less<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >,std::allocator<std::pair<const std::basic_string<char, std::char_traits<char>, std::allocator<char> >, kc::impl_path*> > >", "struct.std::map<std::basic_string<char, std::char_traits<char>, std::allocator<char> >,kc::impl_path*,std::less<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >,std::allocator<std::pair<const std::basic_string<char, std::char_traits<char>, std::allocator<char> >, kc::impl_path*> > >" }

Now, is that signed, or unsigned? :)

The problem is that two types could differ only by sign at some very
complex level of nesting and its entirely possible that variables with
the same name for the two types could be used. I'm trying to peel that
onion slowly so as not to break llvm-upgrade in the process.

More to come.

Reid.



> 
> 
> On 2007-01-02, at 00:44, Reid Spencer wrote:
> 
> > For PR1070: http://llvm.org/PR1070 :
> > 
> > Revise the upgrade parser to keep track of types more faithfully and
> > use
> > 
> > this information to resolve name conflicts resulting from collapsed
> > type
> > 
> > planes. The type planes have collapsed because the integer types are
> > now
> > 
> > signless so that uint and int became i32. Where two planes existed
> > for uint
> > 
> > and int, only i32 exists. Any variable names depending on the type
> > planes
> > 
> > to pmake the identifier unique would cause a conflict. This patch
> > resolves
> > 
> > that conflict for many but not all cases.
> > 
> > 
> > Situations involving the integer types and pointers to them are
> > handled
> > 
> > by this patch.  However, there are corner cases that are not
> > handled 
> > 
> > well, such as:
> > 
> > 
> > %t1 = type { uint, int }
> > 
> > %t2 = type { int, uint }
> > 
> > 
> > void %myfunc(%t1* one, %t2* two) {
> > 
> >   %var = load %t1* one
> > 
> >   %var = load %t2* two
> > 
> > }
> > 
> > 
> > In the scenario above, %t1 and %t2 are really the same type: { i32,
> > i32 }
> > 
> > Consequently attempting to name %var twice will yield a redefinition
> > error
> > 
> > when assembled.  
> > 
> > 
> > While this patch is sufficien to allow the llvm/test suite to pass,
> > More 
> > 
> > work needs to be to complete the handling of these corner cases.
> > 
> 
> 
> 





More information about the llvm-commits mailing list