[llvm] r268452 - PM: Port LoopRotation to the new loop pass manager

[Xinliang David Li via llvm-commits]

On Tue, May 10, 2016 at 11:29 PM, Chandler Carruth <chandlerc at gmail.com>
wrote:

> On Tue, May 10, 2016 at 11:39 PM Xinliang David Li <xinliangli at gmail.com>
> wrote:
>
>>
>>
>> On Tue, May 10, 2016 at 10:08 PM, Justin Bogner <mail at justinbogner.com>
>> wrote:
>>
>>> Xinliang David Li via llvm-commits <llvm-commits at lists.llvm.org> writes:
>>> > On Tue, May 10, 2016 at 4:43 PM, Chandler Carruth via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>> > On Tue, May 10, 2016 at 5:07 PM Sean Silva via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>> >>> Using a function called "getCachedResult" for this purpose seems a
>>> bit
>>> >>> off in the first place. I think it would make sense to call it
>>> >>> `getRequiredResult` or something like that so that it expresses the
>>> >>> semantic intent ("as a loop pass, I can't force this to be computed,
>>> >>> but I require it so it better be there").
>>> >>
>>> >> I think "cached" is still really important -- it signifies that unlike
>>> >> other query paths it won't go and do work all of a sudden. =]
>>> >>
>>> >> That said, I agree that the pattern of requiring a cached result from
>>> an
>>> >> outer pass manager is very common and we should streamline this.
>>> >>
>>> >> The reason I hadn't actually done it is that I'm actually *really*
>>> torn
>>> >> about whether it would be better to have passes just skip their logic
>>> or
>>> >> fail hard. I almost *every* case I prefer the assert or fatal error...
>>> >> except when it comes down to bisecting a failing pass or similar
>>> >> techniques.
>>> >
>>> > How about this:   getCachedResults can either return null or fail hard
>>> > depending on an internal option.  The client code shares the same
>>> boiler plate
>>> > -- skips/bails when null is returned from getCachedResult. In normal
>>> build,
>>> > you would get assertion/fail, but bitsecting tools can disable the
>>> assert.
>>>
>>> That doesn't quite work - there are certainly cases where we use
>>> getCachedResults because it really is optional.
>>>
>>
>> getCachedResults can have an additional parameter indicating whether it
>> is truly required or optional. For optional request, never assert/fail.
>>
>>>
>>> We might be able to combine this idea with adding a second API that
>>> says "I really need this result from the cache and I can't progress
>>> without it".
>>>
>>
>> One interface may suffice -- the additional parameter can default to
>> false (i.e., by default it is not optional).
>>
>
> I really don't like using parameters this way. If we want a different
> semantic model it should have a different API name IMO.
>
> But I don't think we need a switch controlling this and having one doesn't
> actually help any.
>
> 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).
>

By skipping the pass without asserting is basically making it a no-op -- so
why is a switch not helping?


opt -no-fatal-error-missing-required -passes='loop(rotate)'  ... -> no op
opt --passes='loop(rotate)' --> fatal error.



> 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.
>

Having the API implementation to do the asserting/error handling will
likely simplify the client code.

David


>
>
>>
>> David
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160510/71e3e0c0/attachment.html>