Hi Alex, Sanjoy, exposing the internal IRBuilder to users of SCEVExpander violates information hiding, and encourages the tight coupling that makes code bases such as Polly so hard to maintain. SCEVExpander::expandCodeFor returns a Value that (if it's an instruction) can be used to update the insert point of the client's IRBuilder. Is that not enough? No hidden state, no hidden state transitions, so it's locally clear to a reader how the IRBuilder moves. Cheers, Philip 2018-04-29 20:50 GMT+02:00 Sanjoy Das via llvm-dev <llvm-dev at lists.llvm.org> :> Hi Alexandre, > > Sorry I missed this -- I was on vacation when you sent this. > > SCEVExpander already has an IRBuilder in it but AFAICT it isn't > exposed as a public interface. I'd be fine if you wanted to expose a > public `GetIRBuilder()` accessor that let a SCEVExpander client use > the same IRBuilder as SCEVExpander. > > -- Sanjoy > > On Fri, Apr 6, 2018 at 10:55 AM, Alexandre Isoard via llvm-dev > <llvm-dev at lists.llvm.org> wrote: > > Hello, > > > > I use SCEVExpander and IRBuilder to generate some code and I frequently > > end-up breaking dominance because the SCEVExpander insertion point and > the > > IRBuilder insertion point do not advance in synchrony. > > > > Ideally, I could build a SCEVExpander based on an existing IRBuilder (so > > that they move each other). Or even better, SCEVExpander inherit from > > IRBuilder and basically extend it with SCEV expander capabilities (then I > > don't need to use a separate IRBuilder). > > > > Or maybe I am using them wrong? What is the intended way to use those? > > > > -- > > Alexandre Isoard > > > > _______________________________________________ > > LLVM Developers mailing list > > llvm-dev at lists.llvm.org > > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180503/dcae1d47/attachment.html>
Hey, Alternatively, expose a SCEVExpander::getInsertPoint? This would proxy the underlying IRbuilder, with no real state sharing, other than the current insert point. This will be less ugly than recieving the expanded SCEV value, casting to an instruction, and feeding this to the IRBuilder. Cheers, siddharth On Thu 3 May, 2018, 15:36 Philip Pfaffe via llvm-dev, < llvm-dev at lists.llvm.org> wrote:> Hi Alex, Sanjoy, > > exposing the internal IRBuilder to users of SCEVExpander violates > information hiding, and encourages the tight coupling that makes code bases > such as Polly so hard to maintain. SCEVExpander::expandCodeFor returns a > Value that (if it's an instruction) can be used to update the insert point > of the client's IRBuilder. Is that not enough? > > No hidden state, no hidden state transitions, so it's locally clear to a > reader how the IRBuilder moves. > > Cheers, > Philip > > 2018-04-29 20:50 GMT+02:00 Sanjoy Das via llvm-dev < > llvm-dev at lists.llvm.org>: > >> Hi Alexandre, >> >> Sorry I missed this -- I was on vacation when you sent this. >> >> SCEVExpander already has an IRBuilder in it but AFAICT it isn't >> exposed as a public interface. I'd be fine if you wanted to expose a >> public `GetIRBuilder()` accessor that let a SCEVExpander client use >> the same IRBuilder as SCEVExpander. >> >> -- Sanjoy >> >> On Fri, Apr 6, 2018 at 10:55 AM, Alexandre Isoard via llvm-dev >> <llvm-dev at lists.llvm.org> wrote: >> > Hello, >> > >> > I use SCEVExpander and IRBuilder to generate some code and I frequently >> > end-up breaking dominance because the SCEVExpander insertion point and >> the >> > IRBuilder insertion point do not advance in synchrony. >> > >> > Ideally, I could build a SCEVExpander based on an existing IRBuilder (so >> > that they move each other). Or even better, SCEVExpander inherit from >> > IRBuilder and basically extend it with SCEV expander capabilities (then >> I >> > don't need to use a separate IRBuilder). >> > >> > Or maybe I am using them wrong? What is the intended way to use those? >> > >> > -- >> > Alexandre Isoard >> > >> > _______________________________________________ >> > LLVM Developers mailing list >> > llvm-dev at lists.llvm.org >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> > >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-- Sending this from my phone, please excuse any typos! -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180503/2093d8f3/attachment.html>
Hi Siddharth, SCEVExpander::expandCodeFor takes the insert point as an argument, so that's entirely client-controlled. That insert point also never changes. So there is no need to ever query it from the expander. On the other hand, if you wanted to pass around an expander object to expand multiple SCEVs, I think it's much clearer to pass the insert point instead. Again, no hidden state! Cheers, Philip 2018-05-03 12:20 GMT+02:00 Siddharth Bhat <siddu.druid at gmail.com>:> Hey, > > Alternatively, expose a SCEVExpander::getInsertPoint? This would proxy the > underlying IRbuilder, with no real state sharing, other than the current > insert point. > > This will be less ugly than recieving the expanded SCEV value, casting to > an instruction, and feeding this to the IRBuilder. > > Cheers, > siddharth > > > On Thu 3 May, 2018, 15:36 Philip Pfaffe via llvm-dev, < > llvm-dev at lists.llvm.org> wrote: > >> Hi Alex, Sanjoy, >> >> exposing the internal IRBuilder to users of SCEVExpander violates >> information hiding, and encourages the tight coupling that makes code bases >> such as Polly so hard to maintain. SCEVExpander::expandCodeFor returns a >> Value that (if it's an instruction) can be used to update the insert point >> of the client's IRBuilder. Is that not enough? >> >> No hidden state, no hidden state transitions, so it's locally clear to a >> reader how the IRBuilder moves. >> >> Cheers, >> Philip >> >> 2018-04-29 20:50 GMT+02:00 Sanjoy Das via llvm-dev < >> llvm-dev at lists.llvm.org>: >> >>> Hi Alexandre, >>> >>> Sorry I missed this -- I was on vacation when you sent this. >>> >>> SCEVExpander already has an IRBuilder in it but AFAICT it isn't >>> exposed as a public interface. I'd be fine if you wanted to expose a >>> public `GetIRBuilder()` accessor that let a SCEVExpander client use >>> the same IRBuilder as SCEVExpander. >>> >>> -- Sanjoy >>> >>> On Fri, Apr 6, 2018 at 10:55 AM, Alexandre Isoard via llvm-dev >>> <llvm-dev at lists.llvm.org> wrote: >>> > Hello, >>> > >>> > I use SCEVExpander and IRBuilder to generate some code and I frequently >>> > end-up breaking dominance because the SCEVExpander insertion point and >>> the >>> > IRBuilder insertion point do not advance in synchrony. >>> > >>> > Ideally, I could build a SCEVExpander based on an existing IRBuilder >>> (so >>> > that they move each other). Or even better, SCEVExpander inherit from >>> > IRBuilder and basically extend it with SCEV expander capabilities >>> (then I >>> > don't need to use a separate IRBuilder). >>> > >>> > Or maybe I am using them wrong? What is the intended way to use those? >>> > >>> > -- >>> > Alexandre Isoard >>> > >>> > _______________________________________________ >>> > LLVM Developers mailing list >>> > llvm-dev at lists.llvm.org >>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> > >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> >> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> > -- > Sending this from my phone, please excuse any typos! >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180503/dc70427c/attachment.html>