On Fri, Mar 27, 2020 at 1:53 PM Nicolai Hähnle via llvm-dev < llvm-dev at lists.llvm.org> wrote:> On Fri, Mar 27, 2020 at 5:22 PM Chris Lattner via llvm-dev > <llvm-dev at lists.llvm.org> wrote: > > On Mar 27, 2020, at 12:23 AM, Johannes Doerfert < > johannesdoerfert at gmail.com> wrote: > > > > > > > Getting to a multithread clean optimizer is not one bug fix away - > > > this is a multiyear journey, and I would prefer not to see big changes > > > with unclear tradeoffs made in an effort to get a quick win. > > > > I think I missed the suggestion of "big changes with unclear tradeoffs > > made in an effort to get a quick win". So far I thought this was a > > discussion of ideas, methods to gather data, and potential pitfalls. > > > > > > Making use-def chains work differently based on the dynamic type of a > Value* is very problematic to me. It breaks the liskov substitution > principle and is likely to lead to widespread bugs. > > That's why I'm also wary of the idea of just having use lists empty > for certain types without any other special handling. However, I would > argue that if Value::use_begin() etc. contain an assertion that fails > when called on one of the value types that don't have use lists, then > the Liskov substition principle is de facto not broken. It basically > leads to a situation that is as-if Value didn't have use lists in the > first place, and only certain sub-types had use lists. >But it doesn't - it means you write some generic code, test it with some cases & looks like it generalizes to other cases (you don't want to/can't test generic code with all possible generic arguments - that's why substitutability is important) then the code breaks when it hits a constant Value because it doesn't conform to the contract.> If we go down that route, maybe the inheritance hierarchy could be > reorganized in a way that makes that more obvious. > > Cheers, > Nicolai > > > > > > > Instead, let’s break down the problems and fix them (the right way!) > > > one at a time. For example, it seems that the thread agrees that the > > > overall design of constants are a problem: let's talk about > > > (incrementally!) moving constants to the right architecture. There > > > are good suggestions about this upthread. > > > > I'm not sure why you think anyone so far suggested anything to "fix it > > all" or to do it "not-incrementally". For example, I think we are > > talking about the design of constants and how it can be incrementally > > improved (for different purposes). I also think there are interesting > > suggestions upthread but I'm not sure sure there was any kind of > > agreement on "the right architecture”. > > > > > > It sounds like there is agreement that whole module/llvmcontext use-def > chains for constant are a bad idea. There is one specific proposal for how > to fix that that is incremental and avoids the problem I mention above: use > instructions to materialize them, so Constant stops deriving from Value. I > would love to see this be explored as a step along the way, instead of > spending time pondering how bad a break to the core APIs in the compiler > would be in practice. > > > > -Chris > > > > > > _______________________________________________ > > LLVM Developers mailing list > > llvm-dev at lists.llvm.org > > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > > > -- > Lerne, wie die Welt wirklich ist, > aber vergiss niemals, wie sie sein sollte. > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://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/20200327/ebdb168a/attachment.html>
> On Mar 27, 2020, at 1:55 PM, David Blaikie <dblaikie at gmail.com> wrote: > > That's why I'm also wary of the idea of just having use lists empty > for certain types without any other special handling. However, I would > argue that if Value::use_begin() etc. contain an assertion that fails > when called on one of the value types that don't have use lists, then > the Liskov substition principle is de facto not broken. It basically > leads to a situation that is as-if Value didn't have use lists in the > first place, and only certain sub-types had use lists. > > But it doesn't - it means you write some generic code, test it with some cases & looks like it generalizes to other cases (you don't want to/can't test generic code with all possible generic arguments - that's why substitutability is important) then the code breaks when it hits a constant Value because it doesn't conform to the contract.David is exactly right. To say the same thing in another way, such a design point would break library based design, one of the core principles of LLVM that makes it so powerful. To make this explicit, a library that walks use-lists internally would have implicit dependencies in its APIs that some things are not allowed to be constants. -Chris -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200327/6388aca4/attachment.html>
On 3/27/20 11:31 PM, Chris Lattner via llvm-dev wrote:> > >> On Mar 27, 2020, at 1:55 PM, David Blaikie <dblaikie at gmail.com >> <mailto:dblaikie at gmail.com>> wrote: >> >> That's why I'm also wary of the idea of just having use lists empty >> for certain types without any other special handling. However, I >> would >> argue that if Value::use_begin() etc. contain an assertion that fails >> when called on one of the value types that don't have use lists, then >> the Liskov substition principle is de facto not broken. It basically >> leads to a situation that is as-if Value didn't have use lists in the >> first place, and only certain sub-types had use lists. >> >> >> But it doesn't - it means you write some generic code, test it with >> some cases & looks like it generalizes to other cases (you don't want >> to/can't test generic code with all possible generic arguments - >> that's why substitutability is important) then the code breaks when >> it hits a constant Value because it doesn't conform to the contract. > > David is exactly right. To say the same thing in another way, such a > design point would break library based design, one of the core > principles of LLVM that makes it so powerful. > > To make this explicit, a library that walks use-lists internally would > have implicit dependencies in its APIs that some things are not > allowed to be constants. > > -ChrisI'm not sure who is removing me from this discussion but please keep me CCed to it. Thanks, Nick> > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://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/20200327/ef28d6f6/attachment.html>
On Fri, Mar 27, 2020 at 9:55 PM David Blaikie <dblaikie at gmail.com> wrote:> On Fri, Mar 27, 2020 at 1:53 PM Nicolai Hähnle via llvm-dev <llvm-dev at lists.llvm.org> wrote: >> >> On Fri, Mar 27, 2020 at 5:22 PM Chris Lattner via llvm-dev >> <llvm-dev at lists.llvm.org> wrote: >> > On Mar 27, 2020, at 12:23 AM, Johannes Doerfert <johannesdoerfert at gmail.com> wrote: >> > >> > >> > > Getting to a multithread clean optimizer is not one bug fix away - >> > > this is a multiyear journey, and I would prefer not to see big changes >> > > with unclear tradeoffs made in an effort to get a quick win. >> > >> > I think I missed the suggestion of "big changes with unclear tradeoffs >> > made in an effort to get a quick win". So far I thought this was a >> > discussion of ideas, methods to gather data, and potential pitfalls. >> > >> > >> > Making use-def chains work differently based on the dynamic type of a Value* is very problematic to me. It breaks the liskov substitution principle and is likely to lead to widespread bugs. >> >> That's why I'm also wary of the idea of just having use lists empty >> for certain types without any other special handling. However, I would >> argue that if Value::use_begin() etc. contain an assertion that fails >> when called on one of the value types that don't have use lists, then >> the Liskov substition principle is de facto not broken. It basically >> leads to a situation that is as-if Value didn't have use lists in the >> first place, and only certain sub-types had use lists. > > > But it doesn't - it means you write some generic code, test it with some cases & looks like it generalizes to other cases (you don't want to/can't test generic code with all possible generic arguments - that's why substitutability is important) then the code breaks when it hits a constant Value because it doesn't conform to the contract.It's not a break of the Liskov substitution principle, though. For example: In the proposal, you can iterate over uses of an Instruction, and you can iterate over uses of all sub-types of an Instruction. You can't iterate over Constant, but then Constant isn't a sub-type of Instruction. It's a bit of an academic point. What you're really trying to say is that with this particular version of the proposal, it is not **statically** checked at compile-time whether a Value has iterable use lists, and that makes it easier to write incorrect code accidentally. Yes, that's a weakness, and precisely the reason why I suggested that we may want to re-organize the inheritance hierarchy of Values. Cheers, Nicolai> >> >> If we go down that route, maybe the inheritance hierarchy could be >> reorganized in a way that makes that more obvious. >> >> Cheers, >> Nicolai >> >> >> > >> > > Instead, let’s break down the problems and fix them (the right way!) >> > > one at a time. For example, it seems that the thread agrees that the >> > > overall design of constants are a problem: let's talk about >> > > (incrementally!) moving constants to the right architecture. There >> > > are good suggestions about this upthread. >> > >> > I'm not sure why you think anyone so far suggested anything to "fix it >> > all" or to do it "not-incrementally". For example, I think we are >> > talking about the design of constants and how it can be incrementally >> > improved (for different purposes). I also think there are interesting >> > suggestions upthread but I'm not sure sure there was any kind of >> > agreement on "the right architecture”. >> > >> > >> > It sounds like there is agreement that whole module/llvmcontext use-def chains for constant are a bad idea. There is one specific proposal for how to fix that that is incremental and avoids the problem I mention above: use instructions to materialize them, so Constant stops deriving from Value. I would love to see this be explored as a step along the way, instead of spending time pondering how bad a break to the core APIs in the compiler would be in practice. >> > >> > -Chris >> > >> > >> > _______________________________________________ >> > LLVM Developers mailing list >> > llvm-dev at lists.llvm.org >> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >> >> >> -- >> Lerne, wie die Welt wirklich ist, >> aber vergiss niemals, wie sie sein sollte. >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-- Lerne, wie die Welt wirklich ist, aber vergiss niemals, wie sie sein sollte.
On 3/28/20 5:39 AM, Nicolai Hähnle via llvm-dev wrote:> On Fri, Mar 27, 2020 at 9:55 PM David Blaikie <dblaikie at gmail.com> wrote: >> On Fri, Mar 27, 2020 at 1:53 PM Nicolai Hähnle via llvm-dev <llvm-dev at lists.llvm.org> wrote: >>> On Fri, Mar 27, 2020 at 5:22 PM Chris Lattner via llvm-dev >>> <llvm-dev at lists.llvm.org> wrote: >>>> On Mar 27, 2020, at 12:23 AM, Johannes Doerfert <johannesdoerfert at gmail.com> wrote: >>>> >>>> >>>>> Getting to a multithread clean optimizer is not one bug fix away - >>>>> this is a multiyear journey, and I would prefer not to see big changes >>>>> with unclear tradeoffs made in an effort to get a quick win. >>>> I think I missed the suggestion of "big changes with unclear tradeoffs >>>> made in an effort to get a quick win". So far I thought this was a >>>> discussion of ideas, methods to gather data, and potential pitfalls. >>>> >>>> >>>> Making use-def chains work differently based on the dynamic type of a Value* is very problematic to me. It breaks the liskov substitution principle and is likely to lead to widespread bugs. >>> That's why I'm also wary of the idea of just having use lists empty >>> for certain types without any other special handling. However, I would >>> argue that if Value::use_begin() etc. contain an assertion that fails >>> when called on one of the value types that don't have use lists, then >>> the Liskov substition principle is de facto not broken. It basically >>> leads to a situation that is as-if Value didn't have use lists in the >>> first place, and only certain sub-types had use lists. >> >> But it doesn't - it means you write some generic code, test it with some cases & looks like it generalizes to other cases (you don't want to/can't test generic code with all possible generic arguments - that's why substitutability is important) then the code breaks when it hits a constant Value because it doesn't conform to the contract. > It's not a break of the Liskov substitution principle, though. For > example: In the proposal, you can iterate over uses of an Instruction, > and you can iterate over uses of all sub-types of an Instruction. You > can't iterate over Constant, but then Constant isn't a sub-type of > Instruction. > > It's a bit of an academic point. What you're really trying to say is > that with this particular version of the proposal, it is not > **statically** checked at compile-time whether a Value has iterable > use lists, and that makes it easier to write incorrect code > accidentally. Yes, that's a weakness, and precisely the reason why I > suggested that we may want to re-organize the inheritance hierarchy of > Values. > > Cheers, > Nicolai >Nicolai, Please stop removing me from the CC I asked in a previous email to be CCed so I can follow the discussion. Thanks, Nick>>> If we go down that route, maybe the inheritance hierarchy could be >>> reorganized in a way that makes that more obvious. >>> >>> Cheers, >>> Nicolai >>> >>> >>>>> Instead, let’s break down the problems and fix them (the right way!) >>>>> one at a time. For example, it seems that the thread agrees that the >>>>> overall design of constants are a problem: let's talk about >>>>> (incrementally!) moving constants to the right architecture. There >>>>> are good suggestions about this upthread. >>>> I'm not sure why you think anyone so far suggested anything to "fix it >>>> all" or to do it "not-incrementally". For example, I think we are >>>> talking about the design of constants and how it can be incrementally >>>> improved (for different purposes). I also think there are interesting >>>> suggestions upthread but I'm not sure sure there was any kind of >>>> agreement on "the right architecture”. >>>> >>>> >>>> It sounds like there is agreement that whole module/llvmcontext use-def chains for constant are a bad idea. There is one specific proposal for how to fix that that is incremental and avoids the problem I mention above: use instructions to materialize them, so Constant stops deriving from Value. I would love to see this be explored as a step along the way, instead of spending time pondering how bad a break to the core APIs in the compiler would be in practice. >>>> >>>> -Chris >>>> >>>> >>>> _______________________________________________ >>>> LLVM Developers mailing list >>>> llvm-dev at lists.llvm.org >>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> >>> >>> -- >>> Lerne, wie die Welt wirklich ist, >>> aber vergiss niemals, wie sie sein sollte. >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > >
On Sat, Mar 28, 2020 at 2:40 AM Nicolai Hähnle <nhaehnle at gmail.com> wrote:> On Fri, Mar 27, 2020 at 9:55 PM David Blaikie <dblaikie at gmail.com> wrote: > > On Fri, Mar 27, 2020 at 1:53 PM Nicolai Hähnle via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> > >> On Fri, Mar 27, 2020 at 5:22 PM Chris Lattner via llvm-dev > >> <llvm-dev at lists.llvm.org> wrote: > >> > On Mar 27, 2020, at 12:23 AM, Johannes Doerfert < > johannesdoerfert at gmail.com> wrote: > >> > > >> > > >> > > Getting to a multithread clean optimizer is not one bug fix away - > >> > > this is a multiyear journey, and I would prefer not to see big > changes > >> > > with unclear tradeoffs made in an effort to get a quick win. > >> > > >> > I think I missed the suggestion of "big changes with unclear tradeoffs > >> > made in an effort to get a quick win". So far I thought this was a > >> > discussion of ideas, methods to gather data, and potential pitfalls. > >> > > >> > > >> > Making use-def chains work differently based on the dynamic type of a > Value* is very problematic to me. It breaks the liskov substitution > principle and is likely to lead to widespread bugs. > >> > >> That's why I'm also wary of the idea of just having use lists empty > >> for certain types without any other special handling. However, I would > >> argue that if Value::use_begin() etc. contain an assertion that fails > >> when called on one of the value types that don't have use lists, then > >> the Liskov substition principle is de facto not broken. It basically > >> leads to a situation that is as-if Value didn't have use lists in the > >> first place, and only certain sub-types had use lists. > > > > > > But it doesn't - it means you write some generic code, test it with some > cases & looks like it generalizes to other cases (you don't want to/can't > test generic code with all possible generic arguments - that's why > substitutability is important) then the code breaks when it hits a constant > Value because it doesn't conform to the contract. > > It's not a break of the Liskov substitution principle, though.It does in the sense that all Values have a list of uses in the current API - yes, if you're proposing /changing the API/ so that Values don't have a use list API, that's one thing - but adding an assertion/runtime failure is not that. Having a runtime failure where by accessing the uses of /some/ Values is a contract violation/assertion failure/etc - that makes it not substitutable, you can't just treat all Values as equal & have to special case to avoid asking for the uses of some Values. Yes, if you move the use-list API down from Value, so Values don't have use lists, that's different/not what Chris is objecting to, I don't think - it was the " Making use-def chains work differently based on the dynamic type of a Value* is very problematic to me. " that was specifically the objection/liskov breaking issue.> For > example: In the proposal, you can iterate over uses of an Instruction, > and you can iterate over uses of all sub-types of an Instruction. You > can't iterate over Constant, but then Constant isn't a sub-type of > Instruction. > > It's a bit of an academic point. What you're really trying to say is > that with this particular version of the proposal, it is not > **statically** checked at compile-time whether a Value has iterable > use lists, and that makes it easier to write incorrect code > accidentally. Yes, that's a weakness, and precisely the reason why I > suggested that we may want to re-organize the inheritance hierarchy of > Values. > > Cheers, > Nicolai > > > > > >> > >> If we go down that route, maybe the inheritance hierarchy could be > >> reorganized in a way that makes that more obvious. > >> > >> Cheers, > >> Nicolai > >> > >> > >> > > >> > > Instead, let’s break down the problems and fix them (the right way!) > >> > > one at a time. For example, it seems that the thread agrees that > the > >> > > overall design of constants are a problem: let's talk about > >> > > (incrementally!) moving constants to the right architecture. There > >> > > are good suggestions about this upthread. > >> > > >> > I'm not sure why you think anyone so far suggested anything to "fix it > >> > all" or to do it "not-incrementally". For example, I think we are > >> > talking about the design of constants and how it can be incrementally > >> > improved (for different purposes). I also think there are interesting > >> > suggestions upthread but I'm not sure sure there was any kind of > >> > agreement on "the right architecture”. > >> > > >> > > >> > It sounds like there is agreement that whole module/llvmcontext > use-def chains for constant are a bad idea. There is one specific proposal > for how to fix that that is incremental and avoids the problem I mention > above: use instructions to materialize them, so Constant stops deriving > from Value. I would love to see this be explored as a step along the way, > instead of spending time pondering how bad a break to the core APIs in the > compiler would be in practice. > >> > > >> > -Chris > >> > > >> > > >> > _______________________________________________ > >> > LLVM Developers mailing list > >> > llvm-dev at lists.llvm.org > >> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > >> > >> > >> > >> -- > >> Lerne, wie die Welt wirklich ist, > >> aber vergiss niemals, wie sie sein sollte. > >> _______________________________________________ > >> LLVM Developers mailing list > >> llvm-dev at lists.llvm.org > >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > > > -- > Lerne, wie die Welt wirklich ist, > aber vergiss niemals, wie sie sein sollte. >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200328/3f28074f/attachment-0001.html>