[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: include/clang/Basic/Attr.td:2871-2872
   /// distribute: attempt to distribute loop if State == Enable
+  /// pipeline: disable pipelining loop if State == Disable
+  /// pipeline_ii_count: try to pipeline loop for only 'Value' value
 
----------------
alexey.lapshin wrote:
> aaron.ballman wrote:
> > alexey.lapshin wrote:
> > > aaron.ballman wrote:
> > > > Missing full stops at the end of the comments. Also, having read "for only 'Value' value", I'm still not certain what's happening. I think this is a count of some kind, so perhaps "Tries to pipeline 'Values' times." but I don't know what the verb "pipeline" means in this context.
> > > > 
> > > > Are users going to understand what the `ii` means in the user-facing name?
> > > As to ii - yes that should be understood by users, because it is important property of software pipelining optimization. Software Pipelining optimization tries to reorder instructions of original loop(from different iterations) so that resulting loop body takes less cycles. It started from some minimal value of ii and stopped with some maximal value.  i.e. it tries to built schedule for min_ii, then min_ii+1, ... until schedule is found or until max_ii reached.  If resulting value of ii already known then instead of searching in range min_ii, max_ii - it is possible to create schedule for already known ii. 
> > > 
> > > probably following spelling would be better : 
> > > 
> > > pipeline_ii_count: create loop schedule with initiation interval equals 'Value'
> > > because it is important property of software pipelining optimization. 
> > 
> > My point is that I have no idea what "ii" means and I doubt I'll be alone -- does the "ii" differentiate this from other kinds of pipeline loop pragmas we plan to support, or is expected that "pipeline_ii_count" be the only pipeline count option? Can we drop the "ii" and not lose anything?
> > 
> > > pipeline_ii_count: create loop schedule with initiation interval equals 'Value'
> > 
> > equals 'Value' -> equal to 'Value'
> Do you think spelling out  ii would help ? 
> 
> pipeline_initiation_interval(number)
I think it would help. I'm less concerned about the internal names of things than I am for the user-facing identifiers and whether it will be understandable to people other than compiler and optimizer writers.


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

https://reviews.llvm.org/D55710