[PATCH] D52932: [MCSched] Bind PFM Counters to the CPUs instead of the SchedModel.
Clement Courbet via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 5 06:23:40 PDT 2018
courbet added a comment.
@gchatelet a bunch of your comments are on old code touched by unwanted reformatting. I ignored these since I have reverted the formatted changes.
================
Comment at: include/llvm/MC/MCSchedule.h:195
+ // The name of the ProcResource that this counter measures.
+ const char *const ProcResName;
};
----------------
gchatelet wrote:
> What would `ProcResName` be is case of retired instruction for instance?
the retired counter is a scheduler concept, not tied to proc resources. It would go with `CycleCounter` and `UopsCounter` above.
================
Comment at: include/llvm/MC/MCSchedule.h:203
+ static const MCPfmCountersInfo &getDefault() { return Default; }
+ static const MCPfmCountersInfo Default;
};
----------------
gchatelet wrote:
> Why do you need both?
For consistency with MCSchedModel (see getValueForCpu() below).
================
Comment at: lib/MC/MCSchedule.cpp:152
+static_assert(std::is_pod<MCPfmCountersInfo>::value,
+ "We shouldn't have a static constructor here");
+const MCPfmCountersInfo MCPfmCountersInfo::Default = {nullptr, nullptr,
----------------
gchatelet wrote:
> I don't understand the error message.
This is ensuring that the value can be constructed by the linker without running any dynamic initialization.
Repository:
rL LLVM
https://reviews.llvm.org/D52932
More information about the llvm-commits
mailing list