<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Mar 28, 2020 at 2:40 AM Nicolai Hähnle <<a href="mailto:nhaehnle@gmail.com">nhaehnle@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Fri, Mar 27, 2020 at 9:55 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
> On Fri, Mar 27, 2020 at 1:53 PM Nicolai Hähnle via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
>><br>
>> On Fri, Mar 27, 2020 at 5:22 PM Chris Lattner via llvm-dev<br>
>> <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
>> > On Mar 27, 2020, at 12:23 AM, Johannes Doerfert <<a href="mailto:johannesdoerfert@gmail.com" target="_blank">johannesdoerfert@gmail.com</a>> wrote:<br>
>> ><br>
>> ><br>
>> > > Getting to a multithread clean optimizer is not one bug fix away -<br>
>> > > this is a multiyear journey, and I would prefer not to see big changes<br>
>> > > with unclear tradeoffs made in an effort to get a quick win.<br>
>> ><br>
>> > I think I missed the suggestion of "big changes with unclear tradeoffs<br>
>> > made in an effort to get a quick win". So far I thought this was a<br>
>> > discussion of ideas, methods to gather data, and potential pitfalls.<br>
>> ><br>
>> ><br>
>> > 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.<br>
>><br>
>> That's why I'm also wary of the idea of just having use lists empty<br>
>> for certain types without any other special handling. However, I would<br>
>> argue that if Value::use_begin() etc. contain an assertion that fails<br>
>> when called on one of the value types that don't have use lists, then<br>
>> the Liskov substition principle is de facto not broken. It basically<br>
>> leads to a situation that is as-if Value didn't have use lists in the<br>
>> first place, and only certain sub-types had use lists.<br>
><br>
><br>
> 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.<br>
<br>
It's not a break of the Liskov substitution principle, though.</blockquote><div><br>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.<br><br>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.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> For<br>
example: In the proposal, you can iterate over uses of an Instruction,<br>
and you can iterate over uses of all sub-types of an Instruction. You<br>
can't iterate over Constant, but then Constant isn't a sub-type of<br>
Instruction.<br>
<br>
It's a bit of an academic point. What you're really trying to say is<br>
that with this particular version of the proposal, it is not<br>
**statically** checked at compile-time whether a Value has iterable<br>
use lists, and that makes it easier to write incorrect code<br>
accidentally. Yes, that's a weakness, and precisely the reason why I<br>
suggested that we may want to re-organize the inheritance hierarchy of<br>
Values.<br>
<br>
Cheers,<br>
Nicolai<br>
<br>
<br>
><br>
>><br>
>> If we go down that route, maybe the inheritance hierarchy could be<br>
>> reorganized in a way that makes that more obvious.<br>
>><br>
>> Cheers,<br>
>> Nicolai<br>
>><br>
>><br>
>> ><br>
>> > > Instead, let’s break down the problems and fix them (the right way!)<br>
>> > > one at a time.  For example, it seems that the thread agrees that the<br>
>> > > overall design of constants are a problem: let's talk about<br>
>> > > (incrementally!) moving constants to the right architecture. There<br>
>> > > are good suggestions about this upthread.<br>
>> ><br>
>> > I'm not sure why you think anyone so far suggested anything to "fix it<br>
>> > all" or to do it "not-incrementally". For example, I think we are<br>
>> > talking about the design of constants and how it can be incrementally<br>
>> > improved (for different purposes). I also think there are interesting<br>
>> > suggestions upthread but I'm not sure sure there was any kind of<br>
>> > agreement on "the right architecture”.<br>
>> ><br>
>> ><br>
>> > 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.<br>
>> ><br>
>> > -Chris<br>
>> ><br>
>> ><br>
>> > _______________________________________________<br>
>> > LLVM Developers mailing list<br>
>> > <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>> > <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
>><br>
>><br>
>><br>
>> --<br>
>> Lerne, wie die Welt wirklich ist,<br>
>> aber vergiss niemals, wie sie sein sollte.<br>
>> _______________________________________________<br>
>> LLVM Developers mailing list<br>
>> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
<br>
<br>
<br>
-- <br>
Lerne, wie die Welt wirklich ist,<br>
aber vergiss niemals, wie sie sein sollte.<br>
</blockquote></div></div>