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

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin marked 2 inline comments as done.
alexey.lapshin added a comment.

Ok, I will address all issues.



================
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
 
----------------
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'


================
Comment at: include/clang/Basic/DiagnosticParseKinds.td:1192
+def err_pragma_pipeline_invalid_keyword : Error<
+    "invalid argument; expected 'disable'">;
 
----------------
aaron.ballman wrote:
> Can you roll this into the above diagnostic with another `%select`, or does that make it too unreadable?
yes, it makes things too complicated. Though I could do it if necessary.


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

https://reviews.llvm.org/D55710