[llvm] change contents of ScalarEvolution from private to protected (PR #83052)

William Moses via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 27 15:35:51 PST 2024


wsmoses wrote:

For the first, you should be able to just add a flag to SE and plumb that
to corresponding checks.

For the second you should diff the version of overridden methods in
MustExitSE to see the new behavior, and bring that (perhaps feature gates)
into SE. Some of that may need to be cleaned up to meet LLVM standards,
which I can help you with (the PHI part was implemented using a macro iirc
to simplify “rebasing” the overloaded fork).

And thanks again for helping out!


On Tue, Feb 27, 2024 at 6:28 PM Billy Moses ***@***.***> wrote:

> I would probably recommend instead adding a flag to ScalarEvolution which
> tells it to always assume the loop exists, independent of any other checks.
>
> From there the remainder of MustExitScalarEvolution are relatively minor
> features improvements, like looking through phi nodes / unreachable
> handling, which can be contributed in a separate PR.
>
> On Tue, Feb 27, 2024 at 5:58 PM Joshua Ferguson ***@***.***>
> wrote:
>
>> I'm starting to get this implemented. In order to avoid taking the
>> implementation in the wrong direction, I'd like to get feedback whether
>> this is a solution you guys are okay with:
>>
>>    - moving ScalarEvolutionMustExit out of Enzyme and into the headers
>>    and source files for ScalarEvolution, in the llvm namespace but outside of
>>    ScalarEvolution, and moving any necessy utility functions into a new
>>    file under Analysis/Utils/EnzymeFunctionUtils
>>    - declaring ScalarEvolutionMustExit as a friend class within
>>    ScalarEvolution
>>    - changing the instantiation of ScalarEvolutionMustExit to take a
>>    reference to a ScalarEvolution instance, and stores that internally
>>
>> If you guys would prefer this to be handled another way, let me know.
>> totally open to suggestions
>>
>>>> Reply to this email directly, view it on GitHub
>> <https://github.com/llvm/llvm-project/pull/83052#issuecomment-1967851071>,
>> or unsubscribe
>> <https://github.com/notifications/unsubscribe-auth/AAJTUXCAP24TRRH5O2S4PO3YVZQLBAVCNFSM6AAAAABD22UD5WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRXHA2TCMBXGE>
>> .
>> You are receiving this because your review was requested.Message ID:
>> ***@***.***>
>>
>


https://github.com/llvm/llvm-project/pull/83052


More information about the llvm-commits mailing list