[PATCH] D61676: [MCA] Add support for nested and overlapping region markers
Andrea Di Biagio via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu May 9 06:14:53 PDT 2019
andreadb marked 2 inline comments as done.
andreadb added a comment.
In D61676#1496007 <https://reviews.llvm.org/D61676#1496007>, @mattd wrote:
> This looks nice! We should probably also have a test for the case where a user specifies an END before a BEGIN tag.
In that case, llvm-mca would report an error for the END directive.
Correct me if I am wrong, but basically your particular scenario is already covered by `test/tools/llvm-mca/X86/llvm-mca-markers-8.s`.
================
Comment at: docs/CommandGuide/llvm-mca.rst:241
+
+Note that nutliple anonymous regions cannot overlap. Also, overlapping regions
+cannot have the same name.
----------------
mattd wrote:
> s/nutliple/multiple/
I will fix it. Thanks!
================
Comment at: tools/llvm-mca/CodeRegionGenerator.cpp:90
+ // Skip spaces and tabs.
+ Position = Comment.find_first_not_of(" \t");
+ if (Position < Comment.size())
----------------
mattd wrote:
> Perhaps also include colon ':' as a valid delimiter here and in line 82 above.
Do you want to allow something like this?
```
LLVM-MCA-END: foo
```
In case, we can do that in a follow-up patch (if we want to allow it).
I personally think that it is not so important in practice to support that case. But I don't have a strong opinion on it.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61676/new/
https://reviews.llvm.org/D61676
More information about the llvm-commits
mailing list