> On Feb 6, 2015, at 6:40 PM, Chandler Carruth <chandlerc at google.com> wrote: > > On Fri, Feb 6, 2015 at 6:09 PM, Reid Kleckner <rnk at google.com <mailto:rnk at google.com>> wrote: > I think we should keep GEP essentially the same, but disassociate the type being GEPd over from the type of the operands. So, assuming the new ptr type is spelled "ptr", we could use this syntax: > %inner.ptr = getelementptr ptr, ptr %x, i32 1 > > Or if I was adding 1 to a "struct A*" value in C: > %next_elt = getelementptr %struct.A, ptr %x, i32 1 > > Ditto for all other instructions that care about pointee types, like load and store: > %v = load i32, ptr %p ; loads already know (and store!) their loaded type internally > store i32 %v, ptr %p ; no need to duplicate that %p points to, we have the type on %v > > Emphatically agree. No instruction should really change semantics here. GEPs should keep working the exact same way, the type involved should just be separate from the pointer's type. > > > I don't think this can be incremental, I think it all goes at once. > > I have some ideas of how to make it incremental: > > I think you might need to add a new GEP bitcode opcode, since that instruction grows a new type operand that doesn't come from an operand type or result type. > > Yep. And you can add this first, defining the semantics to be as-if the pointer operand was bit casted to a pointer to the new type. Then we could (in theory, not in practice!) even use and test it with the current IR, passing an i8* or any other pointer type operand.I’m not sure we need the new instructions at all. Really what we want is for Load and GEP to carry around a Type* alongside all their operands. This can be added now to the existing Load and GEP insts, and to their constructors/create methods. Going through all of the codebase to ensure we pass in the Type* in all the right places was going to have to be done anyway for a new instruction, but now can be done on the existing instructions. You can also assert (temporarily) to ensure that the type passed in matches the pointer operand type. I do agree the actual final switch could be tricky, but I think we should do what I’ve suggested here first, then see what happens if we just make the switch (which if we get everything in place really just means turning off a bunch of asserts and parser error checks). Personally I think this is far less work and less disruptive to out of tree targets, which are otherwise going to have to find all their own uses of the old style GEP instruction and add another GEP instruction to switches, etc. ...> > Same goes for the load instruction. We could support the new syntax first. > > > Next, I think you might be able to introduce a generic pointer type, spelled as 'ptr' which would verifier fail if used with the old load or gep instructions. You might have to synthesize an unnamable pointee type to use for the in-memory representation, but that seems not beyond reason. Then you can wire up all the asm parsing and printing and bitcode stuff incrementally without any disruption. > > The remaining parts are more interesting and maybe harder to do incrementally, but still seem at least somewhat decomposable: > > - switching all of the LLVM optimizer and all of Clang to produce the new forms of GEP and load rather than the old forms > - switching all of the optimizer and clang to use the new boring pointer type now that they never form old gep and load instructions > - switching all the auto-upgrade functionality on > - removing the in-memory support for the old forms > - simplifying a ton of the in-memory support and the optimizer now that the old forms can't show up > > > Also: > > It also wouldn't be too hard to accept the old .ll syntax, since the upgrade path mostly discards information. > > I agree here. If only because of th eregression test suite, and the *incredible* tediousness of updating the pointers. The auto-upgrade for this kind of thing is essentially perfect.I agree in principle, but I think the precedent here is dangerous. Thanks, Pete> > > -Chandler > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150209/275fe6b0/attachment.html>
On Mon, Feb 9, 2015 at 9:47 AM, Pete Cooper <peter_cooper at apple.com> wrote:> Also: > > It also wouldn't be too hard to accept the old .ll syntax, since the >> upgrade path mostly discards information. >> > > I agree here. If only because of th eregression test suite, and the > *incredible* tediousness of updating the pointers. The auto-upgrade for > this kind of thing is essentially perfect. > > I agree in principle, but I think the precedent here is dangerous.Yea, this is very close to the line. I'm mildly in favor of it, mostly because of the truly pervasive nature of the change and the trivial nature of support.... But it wouldn't be hard to argue me out of it honestly.... -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150209/220a4bea/attachment.html>
On Mon, Feb 9, 2015 at 9:47 AM, Pete Cooper <peter_cooper at apple.com> wrote:> > On Feb 6, 2015, at 6:40 PM, Chandler Carruth <chandlerc at google.com> wrote: > > On Fri, Feb 6, 2015 at 6:09 PM, Reid Kleckner <rnk at google.com> wrote: > >> I think we should keep GEP essentially the same, but disassociate the >> type being GEPd over from the type of the operands. So, assuming the new >> ptr type is spelled "ptr", we could use this syntax: >> %inner.ptr = getelementptr ptr, ptr %x, i32 1 >> >> Or if I was adding 1 to a "struct A*" value in C: >> %next_elt = getelementptr %struct.A, ptr %x, i32 1 >> >> Ditto for all other instructions that care about pointee types, like load >> and store: >> %v = load i32, ptr %p ; loads already know (and store!) their loaded type >> internally >> store i32 %v, ptr %p ; no need to duplicate that %p points to, we have >> the type on %v >> > > Emphatically agree. No instruction should really change semantics here. > GEPs should keep working the exact same way, the type involved should just > be separate from the pointer's type. > > >> >> I don't think this can be incremental, I think it all goes at once. >> > > I have some ideas of how to make it incremental: > > >> I think you might need to add a new GEP bitcode opcode, since that >> instruction grows a new type operand that doesn't come from an operand type >> or result type. >> > > Yep. And you can add this first, defining the semantics to be as-if the > pointer operand was bit casted to a pointer to the new type. Then we could > (in theory, not in practice!) even use and test it with the current IR, > passing an i8* or any other pointer type operand. > > I’m not sure we need the new instructions at all. > > Really what we want is for Load and GEP to carry around a Type* alongside > all their operands. This can be added now to the existing Load and GEP > insts, and to their constructors/create methods. >I'm good with either option (what Chandler described or what Pete's described here). I am actually leaning towards what you've got here, Pete, unless people have objections. At a first pass we wouldn't even need to break LLVM IRBuilder API compatibility - we can take the type from the operand and use that to create the GEP/Load. Add the alternative builder calls that accept the type explicitly and start migrating callers to pass it in explicitly before removing the old version. Frontend test cases would need to be cleaned up up-front, because we'd start generating the new GEP/Load syntax as soon as it was introduced in this way. Then, once GEP/Load (anything else that consumes a pointer & cares about its type?) are done I could introduce the new ptr type, start making instructions be of that type instead - probably at this point I'd spend time cleaning up clang so it doesn't depend on the type of a pointer type at all (I imagine there will be things I can't catch in the first pass even if I do my best to push the types through the stack and not depend on the ones in the pointer operand). Sound plausible?> > Going through all of the codebase to ensure we pass in the Type* in all > the right places was going to have to be done anyway for a new instruction, > but now can be done on the existing instructions. You can also assert > (temporarily) to ensure that the type passed in matches the pointer > operand type. > > I do agree the actual final switch could be tricky, but I think we should > do what I’ve suggested here first, then see what happens if we just make > the switch (which if we get everything in place really just means turning > off a bunch of asserts and parser error checks). > > Personally I think this is far less work and less disruptive to out of > tree targets, which are otherwise going to have to find all their own uses > of the old style GEP instruction and add another GEP instruction to > switches, etc. > > ... > > > Same goes for the load instruction. We could support the new syntax first. > > > Next, I think you might be able to introduce a generic pointer type, > spelled as 'ptr' which would verifier fail if used with the old load or gep > instructions. You might have to synthesize an unnamable pointee type to use > for the in-memory representation, but that seems not beyond reason. Then > you can wire up all the asm parsing and printing and bitcode stuff > incrementally without any disruption. > > The remaining parts are more interesting and maybe harder to do > incrementally, but still seem at least somewhat decomposable: > > - switching all of the LLVM optimizer and all of Clang to produce the new > forms of GEP and load rather than the old forms > - switching all of the optimizer and clang to use the new boring pointer > type now that they never form old gep and load instructions > - switching all the auto-upgrade functionality on > - removing the in-memory support for the old forms > - simplifying a ton of the in-memory support and the optimizer now that > the old forms can't show up > > > Also: > > It also wouldn't be too hard to accept the old .ll syntax, since the >> upgrade path mostly discards information. >> > > I agree here. If only because of th eregression test suite, and the > *incredible* tediousness of updating the pointers. The auto-upgrade for > this kind of thing is essentially perfect. > > I agree in principle, but I think the precedent here is dangerous. > > Thanks, > Pete > > > > -Chandler > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150210/3b8dc2e5/attachment.html>
> On Feb 10, 2015, at 11:02 AM, David Blaikie <dblaikie at gmail.com> wrote: > > > > On Mon, Feb 9, 2015 at 9:47 AM, Pete Cooper <peter_cooper at apple.com <mailto:peter_cooper at apple.com>> wrote: > >> On Feb 6, 2015, at 6:40 PM, Chandler Carruth <chandlerc at google.com <mailto:chandlerc at google.com>> wrote: >> >> On Fri, Feb 6, 2015 at 6:09 PM, Reid Kleckner <rnk at google.com <mailto:rnk at google.com>> wrote: >> I think we should keep GEP essentially the same, but disassociate the type being GEPd over from the type of the operands. So, assuming the new ptr type is spelled "ptr", we could use this syntax: >> %inner.ptr = getelementptr ptr, ptr %x, i32 1 >> >> Or if I was adding 1 to a "struct A*" value in C: >> %next_elt = getelementptr %struct.A, ptr %x, i32 1 >> >> Ditto for all other instructions that care about pointee types, like load and store: >> %v = load i32, ptr %p ; loads already know (and store!) their loaded type internally >> store i32 %v, ptr %p ; no need to duplicate that %p points to, we have the type on %v >> >> Emphatically agree. No instruction should really change semantics here. GEPs should keep working the exact same way, the type involved should just be separate from the pointer's type. >> >> >> I don't think this can be incremental, I think it all goes at once. >> >> I have some ideas of how to make it incremental: >> >> I think you might need to add a new GEP bitcode opcode, since that instruction grows a new type operand that doesn't come from an operand type or result type. >> >> Yep. And you can add this first, defining the semantics to be as-if the pointer operand was bit casted to a pointer to the new type. Then we could (in theory, not in practice!) even use and test it with the current IR, passing an i8* or any other pointer type operand. > I’m not sure we need the new instructions at all. > > Really what we want is for Load and GEP to carry around a Type* alongside all their operands. This can be added now to the existing Load and GEP insts, and to their constructors/create methods. > > I'm good with either option (what Chandler described or what Pete's described here). I am actually leaning towards what you've got here, Pete, unless people have objections. > > At a first pass we wouldn't even need to break LLVM IRBuilder API compatibility - we can take the type from the operand and use that to create the GEP/Load. Add the alternative builder calls that accept the type explicitly and start migrating callers to pass it in explicitly before removing the old version. > > Frontend test cases would need to be cleaned up up-front, because we'd start generating the new GEP/Load syntax as soon as it was introduced in this way. > > Then, once GEP/Load (anything else that consumes a pointer & cares about its type?) are done I could introduce the new ptr type, start making instructions be of that type instead - probably at this point I'd spend time cleaning up clang so it doesn't depend on the type of a pointer type at all (I imagine there will be things I can't catch in the first pass even if I do my best to push the types through the stack and not depend on the ones in the pointer operand). > > Sound plausible?Yeah, sounds great. Thanks, Pete> > > Going through all of the codebase to ensure we pass in the Type* in all the right places was going to have to be done anyway for a new instruction, but now can be done on the existing instructions. You can also assert (temporarily) to ensure that the type passed in matches the pointer operand type. > > I do agree the actual final switch could be tricky, but I think we should do what I’ve suggested here first, then see what happens if we just make the switch (which if we get everything in place really just means turning off a bunch of asserts and parser error checks). > > Personally I think this is far less work and less disruptive to out of tree targets, which are otherwise going to have to find all their own uses of the old style GEP instruction and add another GEP instruction to switches, etc. > > ... >> >> Same goes for the load instruction. We could support the new syntax first. >> >> >> Next, I think you might be able to introduce a generic pointer type, spelled as 'ptr' which would verifier fail if used with the old load or gep instructions. You might have to synthesize an unnamable pointee type to use for the in-memory representation, but that seems not beyond reason. Then you can wire up all the asm parsing and printing and bitcode stuff incrementally without any disruption. >> >> The remaining parts are more interesting and maybe harder to do incrementally, but still seem at least somewhat decomposable: >> >> - switching all of the LLVM optimizer and all of Clang to produce the new forms of GEP and load rather than the old forms >> - switching all of the optimizer and clang to use the new boring pointer type now that they never form old gep and load instructions >> - switching all the auto-upgrade functionality on >> - removing the in-memory support for the old forms >> - simplifying a ton of the in-memory support and the optimizer now that the old forms can't show up >> >> >> Also: >> >> It also wouldn't be too hard to accept the old .ll syntax, since the upgrade path mostly discards information. >> >> I agree here. If only because of th eregression test suite, and the *incredible* tediousness of updating the pointers. The auto-upgrade for this kind of thing is essentially perfect. > I agree in principle, but I think the precedent here is dangerous. > > Thanks, > Pete >> >> >> -Chandler >> >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu <mailto:LLVMdev at cs.uiuc.edu> http://llvm.cs.uiuc.edu <http://llvm.cs.uiuc.edu/> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev <http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev> > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu <mailto:LLVMdev at cs.uiuc.edu> http://llvm.cs.uiuc.edu <http://llvm.cs.uiuc.edu/> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev <http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150210/c8c01e7e/attachment.html>
On Tue, Feb 10, 2015 at 11:02 AM, David Blaikie <dblaikie at gmail.com> wrote:> Sound plausible?+1 from me. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150210/e4eac1e0/attachment.html>
+1 and thank you so much for working on this! On Feb 10, 2015 2:25 PM, "David Blaikie" <dblaikie at gmail.com> wrote:> > > On Mon, Feb 9, 2015 at 9:47 AM, Pete Cooper <peter_cooper at apple.com> > wrote: > >> >> On Feb 6, 2015, at 6:40 PM, Chandler Carruth <chandlerc at google.com> >> wrote: >> >> On Fri, Feb 6, 2015 at 6:09 PM, Reid Kleckner <rnk at google.com> wrote: >> >>> I think we should keep GEP essentially the same, but disassociate the >>> type being GEPd over from the type of the operands. So, assuming the new >>> ptr type is spelled "ptr", we could use this syntax: >>> %inner.ptr = getelementptr ptr, ptr %x, i32 1 >>> >>> Or if I was adding 1 to a "struct A*" value in C: >>> %next_elt = getelementptr %struct.A, ptr %x, i32 1 >>> >>> Ditto for all other instructions that care about pointee types, like >>> load and store: >>> %v = load i32, ptr %p ; loads already know (and store!) their loaded >>> type internally >>> store i32 %v, ptr %p ; no need to duplicate that %p points to, we have >>> the type on %v >>> >> >> Emphatically agree. No instruction should really change semantics here. >> GEPs should keep working the exact same way, the type involved should just >> be separate from the pointer's type. >> >> >>> >>> I don't think this can be incremental, I think it all goes at once. >>> >> >> I have some ideas of how to make it incremental: >> >> >>> I think you might need to add a new GEP bitcode opcode, since that >>> instruction grows a new type operand that doesn't come from an operand type >>> or result type. >>> >> >> Yep. And you can add this first, defining the semantics to be as-if the >> pointer operand was bit casted to a pointer to the new type. Then we could >> (in theory, not in practice!) even use and test it with the current IR, >> passing an i8* or any other pointer type operand. >> >> I’m not sure we need the new instructions at all. >> >> Really what we want is for Load and GEP to carry around a Type* alongside >> all their operands. This can be added now to the existing Load and GEP >> insts, and to their constructors/create methods. >> > > I'm good with either option (what Chandler described or what Pete's > described here). I am actually leaning towards what you've got here, Pete, > unless people have objections. > > At a first pass we wouldn't even need to break LLVM IRBuilder API > compatibility - we can take the type from the operand and use that to > create the GEP/Load. Add the alternative builder calls that accept the type > explicitly and start migrating callers to pass it in explicitly before > removing the old version. > > Frontend test cases would need to be cleaned up up-front, because we'd > start generating the new GEP/Load syntax as soon as it was introduced in > this way. > > Then, once GEP/Load (anything else that consumes a pointer & cares about > its type?) are done I could introduce the new ptr type, start making > instructions be of that type instead - probably at this point I'd spend > time cleaning up clang so it doesn't depend on the type of a pointer type > at all (I imagine there will be things I can't catch in the first pass even > if I do my best to push the types through the stack and not depend on the > ones in the pointer operand). > > Sound plausible? > > >> >> Going through all of the codebase to ensure we pass in the Type* in all >> the right places was going to have to be done anyway for a new instruction, >> but now can be done on the existing instructions. You can also assert >> (temporarily) to ensure that the type passed in matches the pointer >> operand type. >> >> I do agree the actual final switch could be tricky, but I think we should >> do what I’ve suggested here first, then see what happens if we just make >> the switch (which if we get everything in place really just means turning >> off a bunch of asserts and parser error checks). >> >> Personally I think this is far less work and less disruptive to out of >> tree targets, which are otherwise going to have to find all their own uses >> of the old style GEP instruction and add another GEP instruction to >> switches, etc. >> >> ... >> >> >> Same goes for the load instruction. We could support the new syntax first. >> >> >> Next, I think you might be able to introduce a generic pointer type, >> spelled as 'ptr' which would verifier fail if used with the old load or gep >> instructions. You might have to synthesize an unnamable pointee type to use >> for the in-memory representation, but that seems not beyond reason. Then >> you can wire up all the asm parsing and printing and bitcode stuff >> incrementally without any disruption. >> >> The remaining parts are more interesting and maybe harder to do >> incrementally, but still seem at least somewhat decomposable: >> >> - switching all of the LLVM optimizer and all of Clang to produce the new >> forms of GEP and load rather than the old forms >> - switching all of the optimizer and clang to use the new boring pointer >> type now that they never form old gep and load instructions >> - switching all the auto-upgrade functionality on >> - removing the in-memory support for the old forms >> - simplifying a ton of the in-memory support and the optimizer now that >> the old forms can't show up >> >> >> Also: >> >> It also wouldn't be too hard to accept the old .ll syntax, since the >>> upgrade path mostly discards information. >>> >> >> I agree here. If only because of th eregression test suite, and the >> *incredible* tediousness of updating the pointers. The auto-upgrade for >> this kind of thing is essentially perfect. >> >> I agree in principle, but I think the precedent here is dangerous. >> >> Thanks, >> Pete >> >> >> >> -Chandler >> >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150210/d265f767/attachment.html>