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

Thu Dec 20 05:42:46 PST 2018

aaron.ballman added inline comments.

================
Comment at: include/clang/Basic/AttrDocs.td:2673
+
+Check `Software pipelining`, `Modulo scheduling` to get more details on optimisation.
+
----------------
alexey.lapshin wrote:
> aaron.ballman wrote:
> > "Check" -- check where? Perhaps "Search for" but I still don't think that's very satisfying for our documentation as there's no telling what users will find or how it relates (if at all) to this feature.
> > 
> > on optimisation -> on this optimization
> > 
> > I find this sentence doesn't really direct me to much of value (research presentations and class lectures, mostly), so I'd rather see the important details spelled out in our docs so that users don't have to guess as much.
> I am just unsure which references to include here ... links to wikipedia or some concrete resources do not look good. 
> 
> So you are suggesting to spell things out here... Ok, I will prepare short explanation...
> I am just unsure which references to include here ... links to wikipedia or some concrete resources do not look good.

If there's a definitive source for the information, like a research paper or a Wikipedia entry that does a great job explaining it, then feel free to link to it. We can always update dead links when they arise.

> So you are suggesting to spell things out here... Ok, I will prepare short explanation...

If there's not a good place to link to that has the information, then yes, spelling it out would be great. Basically, someone reading these docs should have enough information available (directly or through links) to help them understand the feature even if it doesn't make them an expert in the topic. My concern with the docs right now is that I have no idea what value to put there (Does it have to be positive? Nonzero? Is there an upper bound?) because I can't find an explanation about what that value actually controls.

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

https://reviews.llvm.org/D55710