Page MenuHomePhabricator

add pragmas to control Software Pipelining optimisation
ClosedPublic

Authored by avl on Jan 7 2019, 11:46 AM.

Details

Summary

[PIPELINER] add two pragmas to control Software Pipelining optimisation.

  
#pragma clang loop pipeline(disable)
  
    Disable SWP optimization for the next loop.
    “disable” is the only possible value.
  
#pragma clang loop pipeline_initiation_interval(number)
  
    Set value of initiation interval for SWP
    optimization to specified number value for
    the next loop. Number is the positive value
    greater than 0.
  
These pragmas could be used for debugging or reducing
compile time purposes. It is possible to disable SWP for
concrete loops to save compilation time or to find bugs
by not doing SWP to certain loops. It is possible to set
value of initiation interval to concrete number to save
compilation time by not doing extra pipeliner passes or
to check created schedule for specific initiation interval.

That is llvm part of the fix

Clang part of fix: https://reviews.llvm.org/D55710

Diff Detail

Repository
rL LLVM

Event Timeline

avl created this revision.Jan 7 2019, 11:46 AM
avl updated this revision to Diff 180541.Jan 7 2019, 11:52 AM

make setMII, setMAX_II to be private.

I recently added a WarnMissedTransformationsPass (lib/Transforms/Scalar/WarnMissedTransformations.cpp) which emits a warning if a #pragma clang loop has not been applied, for any reason. It would be nince if it would work for #pragma clang loop as well. For instance, if the target backend does not support yet. My implementation is located after the midend loop passes, so it cannot be adapted directly.

avl added a comment.Jan 9 2019, 5:55 AM

Ok, I will investigate on how to add this functionality.

avl added a comment.Jan 11 2019, 1:11 AM

Michael, I evaluated what should be done to implement warnings about missed transformations for pragma clang loop pipeine. It requires modifications for WarnMissedTransformations pass or creating another one which will work after MIR generated, it would also be neccessary to change metadata handling in MachinePipeliner.cpp. I am OK to do all of these. Though I would prefer to do it as a separate patch.

Would it be OK to integrate this patch in it`s current state and create another patch for warning about missed transformation for #pragma clang loop pipeline ?

In D56403#1353857, @alexey.lapshin wrote:

Would it be OK to integrate this patch in it`s current state and create another patch for warning about missed transformation for #pragma clang loop pipeline ?

Yes.

However, I don't feel qualified for reviewing this patch.

lib/CodeGen/MachinePipeliner.cpp
826 ↗(On Diff #180541)

[nit] unnecessary empty line

859 ↗(On Diff #180541)

[nit] unnecessary empty line

878–880 ↗(On Diff #180541)

[nit] No braces necessary

avl updated this revision to Diff 181758.Jan 15 2019, 2:49 AM

merge with separated MachinePipeliner.h/.cpp and address comments.

bcahoon accepted this revision.Jan 21 2019, 7:53 AM

Just a couple of minor comments on formatting. Otherwise, it looks good to me. Thanks!

include/llvm/CodeGen/MachinePipeliner.h
66 ↗(On Diff #181758)

DisabledByPragma - variable names should start with an upper case letter according to the LLVM Coding Standards.

86 ↗(On Diff #181758)

I don't think you need (or should have) both default member initializer for disabledByPramga/II_setByPragma and a member initializer list. I believe LLVM encourages using the default member initializer.

lib/CodeGen/MachinePipeliner.cpp
365 ↗(On Diff #181758)

no need for braces

This revision is now accepted and ready to land.Jan 21 2019, 7:53 AM
avl added a comment.Jan 21 2019, 9:46 AM

Thank You! Brendon, Could you integrate that fix please ? I do no have commit access yet. As to your comments - how it would be better to handle them - do I need to upload new revision now or just to do them with another fix ?

Hi Alexey,

I can merge the patch for you, and make the minor changes. I encourage you to get commit access, especially if you're going to make more contributions.

Thanks,
Brendon

avl added a comment.Jan 21 2019, 10:33 PM

Hi Brendon,

avl added a comment.Jan 21 2019, 10:35 PM

Hi Brendon,

That would be great!   
Sure, I am going to request commit access. I just need to have several successful patches already integrated for that.

Thank You, Alexey.

This revision was automatically updated to reflect the committed changes.