On Nov 21, 2007, at 6:22 PM, Chris Lattner wrote:> On Wed, 21 Nov 2007, Christopher Lamb wrote: >>> Unlike alignment and volatility, I think that the address space >>> qualifier >>> should be represented explicitly in the type system. The reason >>> for this > >> I've come across a hitch. Store instructions do not reference the >> pointer >> type in the .bc format, only the stored type. The .bc reader >> constructs the >> pointer type from the stored value's type. This means that the >> address space >> information doesn't come along for the ride. > > Ah, that is annoying. :( > >> I see three solutions: >> >> 1) Change how stores are written/read in .bc to store the pointer >> type rather >> than the stored type. This is the most straight forward, but I >> think it also >> breaks .bc compatibility in a way that's impossible to work >> around. There's >> no way to differentiate the new and old forms. > > I strongly prefer this approach. Implementing this without > breaking old > .bc files is actually pretty simple. Just add a new > "FUNC_CODE_INST_STORE2" record, and define it however you want (with a > new, previously unused, ID #). > > The reader should read both FUNC_CODE_INST_STORE (which can't involved > addr spaces) and FUNC_CODE_INST_STORE2 (which can). The .bc writer > can > switch to unconditionally writing out stores in FUNC_CODE_INST_STORE2 > format. > > Please add a generous block comment to > llvm/include/llvm/Bitcode/LLVMBitCodes.h above the new enum explaining > what the difference is though. :)Should I take the same approach to the encoding of pointer types in the pointer table? -- Christopher Lamb -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20071124/7ca3547e/attachment.html>
On Nov 24, 2007, at 7:47 PM, Christopher Lamb wrote:> > On Nov 21, 2007, at 6:22 PM, Chris Lattner wrote: > >> On Wed, 21 Nov 2007, Christopher Lamb wrote: >>>> Unlike alignment and volatility, I think that the address space >>>> qualifier >>>> should be represented explicitly in the type system. The >>>> reason for this >> >>> I've come across a hitch. Store instructions do not reference the >>> pointer >>> type in the .bc format, only the stored type. The .bc reader >>> constructs the >>> pointer type from the stored value's type. This means that the >>> address space >>> information doesn't come along for the ride. >> >> Ah, that is annoying. :( >> >>> I see three solutions: >>> >>> 1) Change how stores are written/read in .bc to store the pointer >>> type rather >>> than the stored type. This is the most straight forward, but I >>> think it also >>> breaks .bc compatibility in a way that's impossible to work >>> around. There's >>> no way to differentiate the new and old forms. >> >> I strongly prefer this approach. Implementing this without >> breaking old >> .bc files is actually pretty simple. Just add a new >> "FUNC_CODE_INST_STORE2" record, and define it however you want >> (with a >> new, previously unused, ID #). >> >> The reader should read both FUNC_CODE_INST_STORE (which can't >> involved >> addr spaces) and FUNC_CODE_INST_STORE2 (which can). The .bc >> writer can >> switch to unconditionally writing out stores in FUNC_CODE_INST_STORE2 >> format. >> >> Please add a generous block comment to >> llvm/include/llvm/Bitcode/LLVMBitCodes.h above the new enum >> explaining >> what the difference is though. :) >Should have said:> Should I take the same approach to the encoding of pointer types in > the types table?-- Christopher Lamb -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20071124/3e4fde9b/attachment.html>
>>> Please add a generous block comment to >>> llvm/include/llvm/Bitcode/LLVMBitCodes.h above the new enum >>> explaining >>> what the difference is though. :) >> > > Should have said: > >> Should I take the same approach to the encoding of pointer types in >> the types table?PointerTypes are a bit easier. The code to write them looks like this: case Type::PointerTyID: // POINTER: [pointee type] Code = bitc::TYPE_CODE_POINTER; TypeVals.push_back(VE.getTypeID(cast<PointerType>(T)- >getElementType())); The code to read them look like this: case bitc::TYPE_CODE_POINTER: // POINTER: [pointee type] if (Record.size() < 1) return Error("Invalid POINTER type record"); ResultTy = PointerType::get(getTypeByID(Record[0], true)); break; The interesting point here is that PointerType currently has one field (that it push_back's onto TypeVals). The reader requires this field to be present, but ignores any additional fields. You should just be able to add another typeVals.push_back() to the writer, and the readers will be transparently compatible with it. Just make the new reader do something like this: case bitc::TYPE_CODE_POINTER: // POINTER: [pointee type] if (Record.size() < 1) return Error("Invalid POINTER type record"); unsigned AddrSpace = Record.size() >= 2 ? Record[1] : 0; ... -Chris