Skip to content

[VPlan] Add additional guiding principles to docs. #85688

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fhahn
Copy link
Contributor

@fhahn fhahn commented Mar 18, 2024

Update VectorizationPlan.rst to include the guiding principles outlined in "VPlan: Status Update and Future Roadmap", LLVM Developers’ Meeting 2023, https://www.youtube.com/watch?v=SzGP4PgMuLE, to the Design Guidelines section.

The newly added points contain references to examples of prior art that illustrate how it is applied in the current codebase.

Preview a rendered version here:
https://gist.github.com/fhahn/c4a6d7121b86904ab5e0fdf30ab54940/raw/45bea52de83f4ab500a534d2627d8d6a101cb390/VectorizationPlan-Guidelines.pdf

Update VectorizationPlan.rst to include the guiding principles outlined
in "VPlan: Status Update and Future Roadmap", LLVM Developers’ Meeting 2023,
https://www.youtube.com/watch?v=SzGP4PgMuLE, to the `Design Guidelines`
section.

The newly added points contain references to examples of prior art that
illustrate how it is applied in the current codebase.

Preview a rendered version here:
https://gist.github.com/fhahn/c4a6d7121b86904ab5e0fdf30ab54940/raw/45bea52de83f4ab500a534d2627d8d6a101cb390/VectorizationPlan-Guidelines.pdf
Copy link
Contributor

@npanchen npanchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That you Florian! Not sure if you planned to use that PR for discussion purposes, but I left few comments to get better clarity.

@@ -88,6 +88,33 @@ The design of VPlan follows several high-level guidelines:
detection and formation involves searching for and optimizing instruction
patterns.

8. The adoption of VPlan components should be done as a gradual, always-on
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I read this correctly, but "always-on refactoring" sounds like "adding ad-hoc changes to adopt some component".
Even though I understand for complex system it's almost impossible to come up with a perfect design that won't change, but should there be at least "perfect at that moment" design that VPlan will pursue with a possible change in mind ?
For example, 3. talks about nested vectorization (multi-level, tenzarization), which is quite complex, but if we consider more down to earth "peel/prolog + main + remainder/epilog/cleanup" loops vectorization, what's expected VPlan representation (just representation, design suppose to include other pieces) ? For instance, it seems to be incorrect to associate VFxUF with a VPlan and will require to introduce it on a VPRegion or something else. My thoughts of this as I expressed in other PR are:

  • VPlan class is Module-like object that contains VPlan IR.
  • VPCountableLoop class is countable loop-like object in HCFG. VFxUF is associated with it and is propagated to each instruction within
  • VPIf class is if-like object in HCFG. Having it outside of the VPCountableLoop treats it as scalar
    ...
    in that case considering simple loop
for (int32_t i = 0; i < e; ++i) {
  red += b[i];
}

for peel + main loops vectorization can be represented as:

VPlan {
  vp.if (<want-to-execute-peel>) {
    vp<%peel.tc> = ...
    vp<%red.initialization> = ...
    vp.loop %i = ir<0>, vp<%peel.tc>, {/*peel vfxuf candidates*/} {
        %red.peel = phi [vp<%red.initialization>, vp<%red.peel.next>] 
        = gep
        = load
        %red.peel.next = 
    }
  }
  vp<%main.start> = phi [0, vp<%peel.tc>]
  vp<%main.tc> = 
  vp.loop %i = vp<%main.start>, vp<%main.tc> {/*main vfxuf candidates*/} {
      %red = phi [vp<%red.peel.next>, vp<%red.next>] 
      = gep
      = load
      %red.next = 
  }
  ir<%red> = reduce %red.next
  vp.loop %i = vp<%main.tc>, ir<N> {scalar} {
      %red.scalar = phi [ir<%red.scalar.next>, ir<%red>] 
      = gep
      = load
      %red.scalar.next = 
  }
};

That said, having couple different examples of "ideal" representation of cases that VPlan should eventually address in the document will be great.

@@ -88,6 +88,33 @@ The design of VPlan follows several high-level guidelines:
detection and formation involves searching for and optimizing instruction
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: cannot comment on a line 86, but I assume Step 2.b should really be Step 2.ii

8. The adoption of VPlan components should be done as a gradual, always-on
refactoring to retain quality and integrate continuously.

9. Gradually refactor into multiple VPlan-to-VPlan transforms to reduce
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please elaborate "reduce complexity" part ? Is it just complexity in terms of building and executing VPlan or something else ?
Essentially, I'm missing what is must and what's nice-to in VPlan. At least my understanding that all transformations in VPlanTransforms, except for VPInstructionsToVPRecipes, are optional.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly rest of it, i.e. initial VPlan should be constructed as-is and all optimizations on it should be done by transforms (optimizations), that looks good. However, I'm also trying to map that point to #82270. Can you please also elaborate which of these principles explains moving canonical iv construction to prepareToExecute ?

the early stages of the pipeline, which later get expanded to replicate
regions; this simplifes a number of transformations, including sinking, by
avoiding having to deal with replicate regions).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple more questions/clarifications I'd like to ask:

  • Is there a goal to be able to reuse LLVM IR's utils/analyses as much as possible to do VPlan's transformations ? There are cases when that might be beneficial to align API with LLVM's Instructions in future. Currently the only one API I find missing is VPValue::getType(), especially in during initial VPlan construction
  • Is it worth to care about downstream compilers ? There are changes we for sure want to upstream, but there could be changes that impossible/hard to upstream. Having VPlan's infrastructure easy-to-extend/change for downstream compilers seems to be a nice-to-have in that document

@npanchen
Copy link
Contributor

@fhahn ping

@fhahn
Copy link
Contributor Author

fhahn commented Apr 12, 2024

@fhahn ping

Thanks for the reminder, still catching up after EuroLLVM, but should hopefully have an update next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants