[llvm] r324582 - [TargetSchedule] Expose sub-units of a ProcResGroup in MCProcResourceDesc.
Clement Courbet via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 26 04:30:57 PST 2018
Hi Andrea, sorry I missed your email.
On Mon, Feb 19, 2018 at 6:23 PM, Andrea Di Biagio <andrea.dibiagio at gmail.com
> wrote:
> Hi Clement,
>
> Sorry for the late reply. I have been off sick for a while and I've only
> just seen your commit now I'm back to work.
>
> I am currently working on an LLVM based performance analysis tool which
> reuses the information from the scheduling model, for which I plan to send
> an RFC soon (next week hopefully).
>
Interesting, I'm working on a similar thing and I'm going to give a talk
about it at EuroLLVM next month. Please keep me in the loop :)
> When working on my tool, I had to solve a similar problem to this, and had
> come up with a different fix which doesn't require generating an extra
> table with indices.
>
> My implementation (for which I'm happy to share the patch) uses simple
> bitmasks to solve set membership problems. The idea is that each processor
> resource is associated with a bitmask value (a 64-bit unisigned quantity)
> which can be used as a unique identifier.
> [...]
> I tried to look to see if there is any specific reason why this approach
> was taken in the review, but I couldn't see it there. If there is not a
> specific reason to use the table approach, would you be okay if we adopted
> the bitmask approach?
>
The reason I went in that direction is the 64 resource limit; my initial
approach was the same as yours. I don't have strong feelings about which is
better.
I agree with the set operations being easier (my tool actually
uses BitVector internally), on the other the table approach is more general
and having an extra table hurts because it's super tiny.
I have no issue going in the bitmask direction if that works better for you
and other people; the semantics are the same.
Feel free to send me a diff.
> To be clear, I am not suggesting that your approach is wrong. However, I
> just want to point out that there may be potentially better approaches.
> The bitmask approach makes the implementation of my tool easier (it would
> make my life easier :-)).
>
> Thanks,
> Andrea
>
> On Fri, Feb 9, 2018 at 8:17 AM, Clement Courbet via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Yes, thanks for pointing that out.
>>
>> On Thu, Feb 8, 2018 at 7:33 PM, Craig Topper <craig.topper at gmail.com>
>> wrote:
>>
>>> Is this responsible for this warning I'm seeing now
>>>
>>> lib/Target/X86/X86GenSubtargetInfo.inc:52484:39: warning: missing field
>>> 'SubUnitsIdxBegin' initializer [-Wmissing-field-initializers]
>>> {DBGFIELD("InvalidUnit") 0, 0, 0},
>>>
>>> ~Craig
>>>
>>> On Thu, Feb 8, 2018 at 12:46 AM, Clement Courbet via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>> Author: courbet
>>>> Date: Thu Feb 8 00:46:48 2018
>>>> New Revision: 324582
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=324582&view=rev
>>>> Log:
>>>> [TargetSchedule] Expose sub-units of a ProcResGroup in
>>>> MCProcResourceDesc.
>>>>
>>>> Summary:
>>>> Right now using a ProcResource automatically counts as usage of all
>>>> super ProcResGroups. All this is done during codegen, so there is no
>>>> way for schedulers to get this information at runtime.
>>>>
>>>> This adds the information of which individual ProcRes units are
>>>> contained in a ProcResGroup in MCProcResourceDesc.
>>>>
>>>> Reviewers: gchatelet
>>>>
>>>> Subscribers: llvm-commits
>>>>
>>>> Differential Revision: https://reviews.llvm.org/D43023
>>>>
>>>> Modified:
>>>> llvm/trunk/include/llvm/MC/MCSchedule.h
>>>> llvm/trunk/include/llvm/Target/TargetSchedule.td
>>>> llvm/trunk/utils/TableGen/SubtargetEmitter.cpp
>>>>
>>>> Modified: llvm/trunk/include/llvm/MC/MCSchedule.h
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/
>>>> MC/MCSchedule.h?rev=324582&r1=324581&r2=324582&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- llvm/trunk/include/llvm/MC/MCSchedule.h (original)
>>>> +++ llvm/trunk/include/llvm/MC/MCSchedule.h Thu Feb 8 00:46:48 2018
>>>> @@ -44,6 +44,11 @@ struct MCProcResourceDesc {
>>>> // an out-of-order cpus.
>>>> int BufferSize;
>>>>
>>>> + // If the resource has sub-units, a pointer to the first element of
>>>> an array
>>>> + // of `NumUnits` elements containing the ProcResourceIdx of the sub
>>>> units.
>>>> + // nullptr if the resource does not have sub-units.
>>>> + const unsigned *SubUnitsIdxBegin;
>>>> +
>>>> bool operator==(const MCProcResourceDesc &Other) const {
>>>> return NumUnits == Other.NumUnits && SuperIdx == Other.SuperIdx
>>>> && BufferSize == Other.BufferSize;
>>>>
>>>> Modified: llvm/trunk/include/llvm/Target/TargetSchedule.td
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/
>>>> Target/TargetSchedule.td?rev=324582&r1=324581&r2=324582&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- llvm/trunk/include/llvm/Target/TargetSchedule.td (original)
>>>> +++ llvm/trunk/include/llvm/Target/TargetSchedule.td Thu Feb 8
>>>> 00:46:48 2018
>>>> @@ -175,8 +175,7 @@ class ProcResourceKind;
>>>> // BufferSize=1.
>>>> //
>>>> // SchedModel ties these units to a processor for any stand-alone defs
>>>> -// of this class. Instances of subclass ProcResource will be
>>>> automatically
>>>> -// attached to a processor, so SchedModel is not needed.
>>>> +// of this class.
>>>> class ProcResourceUnits<ProcResourceKind kind, int num> {
>>>> ProcResourceKind Kind = kind;
>>>> int NumUnits = num;
>>>>
>>>> Modified: llvm/trunk/utils/TableGen/SubtargetEmitter.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGe
>>>> n/SubtargetEmitter.cpp?rev=324582&r1=324581&r2=324582&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- llvm/trunk/utils/TableGen/SubtargetEmitter.cpp (original)
>>>> +++ llvm/trunk/utils/TableGen/SubtargetEmitter.cpp Thu Feb 8 00:46:48
>>>> 2018
>>>> @@ -92,6 +92,8 @@ class SubtargetEmitter {
>>>> &ProcItinLists);
>>>> void EmitProcessorProp(raw_ostream &OS, const Record *R, StringRef
>>>> Name,
>>>> char Separator);
>>>> + void EmitProcessorResourceSubUnits(const CodeGenProcModel
>>>> &ProcModel,
>>>> + raw_ostream &OS);
>>>> void EmitProcessorResources(const CodeGenProcModel &ProcModel,
>>>> raw_ostream &OS);
>>>> Record *FindWriteResources(const CodeGenSchedRW &SchedWrite,
>>>> @@ -578,24 +580,52 @@ void SubtargetEmitter::EmitProcessorProp
>>>> OS << '\n';
>>>> }
>>>>
>>>> +void SubtargetEmitter::EmitProcessorResourceSubUnits(
>>>> + const CodeGenProcModel &ProcModel, raw_ostream &OS) {
>>>> + OS << "\nstatic const unsigned " << ProcModel.ModelName
>>>> + << "ProcResourceSubUnits[] = {\n"
>>>> + << " 0, // Invalid\n";
>>>> +
>>>> + for (unsigned i = 0, e = ProcModel.ProcResourceDefs.size(); i < e;
>>>> ++i) {
>>>> + Record *PRDef = ProcModel.ProcResourceDefs[i];
>>>> + if (!PRDef->isSubClassOf("ProcResGroup"))
>>>> + continue;
>>>> + RecVec ResUnits = PRDef->getValueAsListOfDefs("Resources");
>>>> + for (Record *RUDef : ResUnits) {
>>>> + Record *const RU =
>>>> + SchedModels.findProcResUnits(RUDef, ProcModel,
>>>> PRDef->getLoc());
>>>> + for (unsigned J = 0; J < RU->getValueAsInt("NumUnits"); ++J) {
>>>> + OS << " " << ProcModel.getProcResourceIdx(RU) << ", ";
>>>> + }
>>>> + }
>>>> + OS << " // " << PRDef->getName() << "\n";
>>>> + }
>>>> + OS << "};\n";
>>>> +}
>>>> +
>>>> void SubtargetEmitter::EmitProcessorResources(const CodeGenProcModel
>>>> &ProcModel,
>>>> raw_ostream &OS) {
>>>> - OS << "\n// {Name, NumUnits, SuperIdx, IsBuffered}\n";
>>>> + EmitProcessorResourceSubUnits(ProcModel, OS);
>>>> +
>>>> + OS << "\n// {Name, NumUnits, SuperIdx, IsBuffered,
>>>> SubUnitsIdxBegin}\n";
>>>> OS << "static const llvm::MCProcResourceDesc "
>>>> << ProcModel.ModelName << "ProcResources" << "[] = {\n"
>>>> << " {DBGFIELD(\"InvalidUnit\") 0, 0, 0},\n";
>>>>
>>>> + unsigned SubUnitsOffset = 1;
>>>> for (unsigned i = 0, e = ProcModel.ProcResourceDefs.size(); i < e;
>>>> ++i) {
>>>> Record *PRDef = ProcModel.ProcResourceDefs[i];
>>>>
>>>> Record *SuperDef = nullptr;
>>>> unsigned SuperIdx = 0;
>>>> unsigned NumUnits = 0;
>>>> + const unsigned SubUnitsBeginOffset = SubUnitsOffset;
>>>> int BufferSize = PRDef->getValueAsInt("BufferSize");
>>>> if (PRDef->isSubClassOf("ProcResGroup")) {
>>>> RecVec ResUnits = PRDef->getValueAsListOfDefs("Resources");
>>>> for (Record *RU : ResUnits) {
>>>> NumUnits += RU->getValueAsInt("NumUnits");
>>>> + SubUnitsOffset += NumUnits;
>>>> }
>>>> }
>>>> else {
>>>> @@ -612,8 +642,14 @@ void SubtargetEmitter::EmitProcessorReso
>>>> OS << " {DBGFIELD(\"" << PRDef->getName() << "\") ";
>>>> if (PRDef->getName().size() < 15)
>>>> OS.indent(15 - PRDef->getName().size());
>>>> - OS << NumUnits << ", " << SuperIdx << ", "
>>>> - << BufferSize << "}, // #" << i+1;
>>>> + OS << NumUnits << ", " << SuperIdx << ", " << BufferSize << ", ";
>>>> + if (SubUnitsBeginOffset != SubUnitsOffset) {
>>>> + OS << ProcModel.ModelName << "ProcResourceSubUnits + "
>>>> + << SubUnitsBeginOffset;
>>>> + } else {
>>>> + OS << "nullptr";
>>>> + }
>>>> + OS << "}, // #" << i+1;
>>>> if (SuperDef)
>>>> OS << ", Super=" << SuperDef->getName();
>>>> OS << "\n";
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>>
>>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180226/6d3fbde2/attachment-0001.html>
More information about the llvm-commits
mailing list