hfinkel 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 ---------------- aaron.ballman wrote: > 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. Yes, I also think that it would help to spell this out. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55710/new/ https://reviews.llvm.org/D55710 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
