[PATCH] D80580: Separate Peeling Properties into its own struct

Tue May 26 14:11:18 PDT 2020

fhahn added a comment.

In D80580#2055795 <https://reviews.llvm.org/D80580#2055795>, @sidbav wrote:

> In D80580#2055720 <https://reviews.llvm.org/D80580#2055720>, @fhahn wrote:
>
> > Could you elaborate why this change is desired? Is there a problem with getting the unrolling preferences and checking the peeling properties in that way? If that's the case it would be good to provide some context (e.g. in form of a follow-on patch that uses the new PeelingPreferences).
>
>
> I would not really call it a problem, but more of an inconvenience. Peeling should not be attached to Unrolling if other Transforms can also make use of peeling. Additionally when another transformation is accessing the peeling properties, I do not think it would make sense to do so through the UnrollingPreferences struct. This could make for some quite confusing code.
>
> However, I can start to look into a follow-up patch which makes use of the PeelingPreferences struct.

Ok great, it would be good to show a clear benefit, as this change has the potential to add some churn for out-of-tree maintainers/targets, which is fine, if there's a clear benefit.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80580/new/

https://reviews.llvm.org/D80580