[PATCH] D63040: [Docs] [llvm-mca] Point out a caveat for using llvm-mca markers in source code.

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

Hi Max,

Thanks for the patch.

I am of the opinion that the change should be much smaller than this. See my commets below.

Cheers
-Andrea



================
Comment at: llvm/docs/CommandGuide/llvm-mca.rst:244-246
+USING MARKERS IN SOURCE CODE
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
----------------
I don't think that we should introduce a subsection here. I would rather keep the description of source level markers short and still as part of the same section.



================
Comment at: llvm/docs/CommandGuide/llvm-mca.rst:260-263
+However, currently, `this interferes with optimizations like loop vectorization
+<https://bugs.llvm.org/show_bug.cgi?id=42173>`_ and changes the code that is generated.
+This is because the ``__asm`` statements are seen as real code having important side
+effects, which limits how the code around them can be transformed.
----------------
Please remove the reference to the upstream bugzilla. I don't think that we should add references to upstream bugs in this document.

Please change "changes the code that is generated" to "may have an impact on the code generated".


================
Comment at: llvm/docs/CommandGuide/llvm-mca.rst:265-269
+It is generally safe to use source-level markers in straight-line source code without
+any control flow, like the example above.  Otherwise, you must take care to make
+sure that the code you are analyzing is the same as the code the compiler actually
+wants to generate.  Unfortunately, the only robust way to do this is to manually
+add the markers to the assembly text.
----------------
The impact of source level markers on the compiler generated code is not quantifiable. It really depends on a number of things. So we shouldn't claim that there are cases where it is generally safe. It is misleading and inaccurate at best.

Even wanting to keep that first statement, it is unclear what you meant by "source-level markers in straight-line source code without any control flow". That is going to cause a lot of confusion to the reader (which knows already that a code sequence should never span through multiple basic blocks) and we don't want that.

Strictly speaking, the entire paragraph can be summarized as "use markers at your own risk", which is clearly already impled by your previous paragraph. So, to be honest it is a bit of a repetition.
In my opinion you should remove this entire paragraph, as it is redundant.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63040