MaskRay added inline comments.
================
Comment at: llvm/lib/Support/TimeProfiler.cpp:77
+static std::string getThreadName() {
+ SmallString<64> Name;
----------------
broadwaylamb wrote:
> MaskRay wrote:
> > You can define ThreadName as `SmallString<0>` to avoid this function.
> Then the `ThreadName` field will lose `const`-ness, and I will also need to
> call `llvm::get_thread_name` in the constructor body, not the member
> initialization list. Would that be OK?
This is OK. Thanks
================
Comment at: llvm/lib/Support/TimeProfiler.cpp:80
+ llvm::get_thread_name(Name);
+ return std::string(std::move(Name));
+}
----------------
`return Name.str();`
================
Comment at: llvm/lib/Support/TimeProfiler.cpp:249
+
+ for (const auto &TTP : ThreadTimeTraceProfilerInstances) {
+ writeMetadataEvent("thread_name", TTP->Tid, TTP->ThreadName);
----------------
broadwaylamb wrote:
> MaskRay wrote:
> > Delete `{}` for simple statements. In LLVM code, such `{}` is not common.
> The code above uses `{}`.
>
> According to http://llvm.org/docs/CodingStandards.html:
>
> >If you are extending, enhancing, or bug fixing already implemented code, use
> >the style that is already being used so that the source is uniform and easy
> >to follow.
They were added without enough scrutiny. There are many other simple statements
in this file not surrounded by `{}`
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78027/new/
https://reviews.llvm.org/D78027
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
