<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 10, 2016 at 11:29 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><div class="h5"><div dir="ltr">On Tue, May 10, 2016 at 11:39 PM Xinliang David Li <<a href="mailto:xinliangli@gmail.com" target="_blank">xinliangli@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><img src="https://ssl.gstatic.com/ui/v1/icons/mail/images/cleardot.gif"><br><div class="gmail_extra"><br><div class="gmail_quote"></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, May 10, 2016 at 10:08 PM, Justin Bogner <span dir="ltr"><<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>Xinliang David Li via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> writes:<br>
> On Tue, May 10, 2016 at 4:43 PM, Chandler Carruth via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
> On Tue, May 10, 2016 at 5:07 PM Sean Silva via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
</span><span>>>> Using a function called "getCachedResult" for this purpose seems a bit<br>
>>> off in the first place. I think it would make sense to call it<br>
>>> `getRequiredResult` or something like that so that it expresses the<br>
>>> semantic intent ("as a loop pass, I can't force this to be computed,<br>
>>> but I require it so it better be there").<br>
>><br>
>> I think "cached" is still really important -- it signifies that unlike<br>
>> other query paths it won't go and do work all of a sudden. =]<br>
>><br>
>> That said, I agree that the pattern of requiring a cached result from an<br>
>> outer pass manager is very common and we should streamline this.<br>
>><br>
>> The reason I hadn't actually done it is that I'm actually *really* torn<br>
>> about whether it would be better to have passes just skip their logic or<br>
>> fail hard. I almost *every* case I prefer the assert or fatal error...<br>
>> except when it comes down to bisecting a failing pass or similar<br>
>> techniques.<br>
><br>
> How about this: getCachedResults can either return null or fail hard<br>
> depending on an internal option. The client code shares the same boiler plate<br>
> -- skips/bails when null is returned from getCachedResult. In normal build,<br>
> you would get assertion/fail, but bitsecting tools can disable the assert.<br>
<br>
</span>That doesn't quite work - there are certainly cases where we use<br>
getCachedResults because it really is optional.<br></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>getCachedResults can have an additional parameter indicating whether it is truly required or optional. For optional request, never assert/fail. </div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
We might be able to combine this idea with adding a second API that<br>
says "I really need this result from the cache and I can't progress<br>
without it".<br></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>One interface may suffice -- the additional parameter can default to false (i.e., by default it is not optional).</div></div></div></div></blockquote><div><br></div></div></div><div>I really don't like using parameters this way. If we want a different semantic model it should have a different API name IMO.</div><div><br></div><div>But I don't think we need a switch controlling this and having one doesn't actually help any.</div><div><br></div><div>For pass pipeline random exploration and such, you really need adding the loop pass without the analysis is a no-op as opposed to an error (whether assert or otherwise).</div></div></div></blockquote><div><br></div><div>By skipping the pass without asserting is basically making it a no-op -- so why is a switch not helping?</div><div> </div><div><br></div><div>opt -no-fatal-error-missing-required -passes='loop(rotate)' ... -> no op</div><div>opt --passes='loop(rotate)' --> fatal error.</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><div>But maybe with a good API for handling the fatal errors and by not using assert at all but report_fatal_error we'll be able to do more fuzzing-style exploration by detecting the failures. It doesn't seem like a fantastic solution, but not the worst. And I don't think we're painting ourselves into a corner here so I'm not too worried about it.</div></div></div></blockquote><div><br></div><div>Having the API implementation to do the asserting/error handling will likely simplify the client code.</div><div><br></div><div>David</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>David</div><div> </div></div></div></div></blockquote></div></div>
</blockquote></div><br></div></div>