[llvm-commits] [llvm] r44858 - in /llvm/trunk: include/llvm/Bitcode/LLVMBitCodes.h include/llvm/DerivedTypes.h include/llvm/GlobalVariable.h include/llvm/Instructions.h lib/AsmParser/LLLexer.cpp lib/AsmParser/llvmAsmParser.y lib/Bitcode/Reader/BitcodeReader.cpp lib/Bitcode/Writer/BitcodeWriter.cpp lib/VMCore/AsmWriter.cpp lib/VMCore/Constants.cpp lib/VMCore/Globals.cpp lib/VMCore/Instructions.cpp lib/VMCore/Type.cpp test/Assembler/2007-12-11-AddressSpaces.ll tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp

[Chris Lattner]

>> Making the address space default to zero is convenient, but
>> dangerous.  This means that xforms that play with pointers need to be
>> very careful to propagate this info in some cases.
>
> This is true.
>
>> Do you think this  is the best way to go?
>
> As opposed to no default value, forcing clients to propagate this  
> information?

Yep.

>> Do many clients of PointerType::get need to  be aware of addr spaces?
>
> Many do. Unfortunately it seems common to simply pass a value's type  
> around and synthesize a pointer when needed. The addition of address  
> spaces means that this approach is no longer equivalent to passing  
> the pointer type, and I expect there will be fixes needed to once a  
> back end depends on this feature.

I think I'd really rather have people have to provide it so that  
people cons'ing up pointers have to think about alternate address  
spaces and that all existing code gets updated.  This makes it much  
less likely that alternate address spaces are a second class feature  
that is just too buggy to rely on.

>> It would be nice to use the above stuff to avoid duplicating this
>> production.  Maybe it would need to be:
>>
>> OptCommaAddrSpace ::= ',' ADDRSPACE '(' EUINT64VAL ')' { $$=$4; }
>> OptCommaAddrSpace ::= /*empty*/ { $$ = 0; }
>>
>> And then just add OptCommaAddrSpace to the existing production.  What
>> do you think?
>
> Avoiding the duplicate production is good, but I'm not sure about  
> the comma. The addrspace() qualifier can not be positionally  
> permuted with other attributes, the comma gives the impression that  
> it can be.

Good point, but I don't think lack of comma really makes it clear  
either...

-Chris