[PATCH] D33352: [globalisel][tablegen] Fix incorrect inclusion of VS_32/VS_64 in [VS]GPRRegBank
Daniel Sanders via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri May 19 04:49:39 PDT 2017
dsanders created this revision.
Herald added subscribers: tpr, igorb, kristof.beyls, rengolin, aemerson.
Re-worked the code that adds register classes to register banks to fix a bug
that occurs when subregister indices are re-used for distinct register classes.
This occurred on AMDGPU and caused VS_32/VS_64 to be incorrectly added to both
VGPRRegBank and SGPRRegBank.
Previously the code would inspect a register class (X) that is known to be in
the register bank and check whether it implied that another register bank (Y)
should also be a member. This code would consider Y to be covered if every
register was accessible by a subregister index that was used by X to access
subregisters. However, it did not check whether the subregisters of X were
present in Y.
This worked fine for AArch64, ARM, and X86 but on AMDGPU it was possible for
the subregisters of X to be missing from Y due to the re-use of subregister
indices.
Now the code checks that every register in Y is either in X or is a
subregister of a register in X.
This has also has a side-effect on X86. It now notices that GR64_TCW64
should be in GPRRegBank.
https://reviews.llvm.org/D33352
Files:
utils/TableGen/RegisterBankEmitter.cpp
Index: utils/TableGen/RegisterBankEmitter.cpp
===================================================================
--- utils/TableGen/RegisterBankEmitter.cpp
+++ utils/TableGen/RegisterBankEmitter.cpp
@@ -180,30 +180,43 @@
VisitFn(RC, Kind.str());
for (const auto &PossibleSubclass : RegisterClassHierarchy.getRegClasses()) {
+ if (RC == &PossibleSubclass)
+ continue;
+
std::string TmpKind =
- (Twine(Kind) + " (" + PossibleSubclass.getName() + ")").str();
+ (Twine(Kind) + " (then, while considering possible-subclass=" +
+ PossibleSubclass.getName())
+ .str();
// Visit each subclass of an explicitly named class.
- if (RC != &PossibleSubclass && RC->hasSubClass(&PossibleSubclass))
+ if (RC->hasSubClass(&PossibleSubclass))
visitRegisterBankClasses(RegisterClassHierarchy, &PossibleSubclass,
- TmpKind + " " + RC->getName() + " subclass",
+ TmpKind + ") and " + RC->getName() +
+ " is a subclass",
VisitFn, VisitedRCs);
- // Visit each class that contains only subregisters of RC with a common
- // subregister-index.
- //
- // More precisely, PossibleSubclass is a subreg-class iff Reg:SubIdx is in
- // PossibleSubclass for all registers Reg from RC using any
- // subregister-index SubReg
- for (const auto &SubIdx : RegisterClassHierarchy.getSubRegIndices()) {
- BitVector BV(RegisterClassHierarchy.getRegClasses().size());
- PossibleSubclass.getSuperRegClasses(&SubIdx, BV);
- if (BV.test(RC->EnumValue)) {
- std::string TmpKind2 = (Twine(TmpKind) + " " + RC->getName() +
- " class-with-subregs: " + RC->getName())
- .str();
- VisitFn(&PossibleSubclass, TmpKind2);
- }
+ // Visit each class that contains only subregisters of RC. The
+ // subregister-index can vary between them such.
+ SmallPtrSet<const CodeGenRegister *, 32> SubRegs = {
+ PossibleSubclass.getMembers().begin(),
+ PossibleSubclass.getMembers().end()};
+ for (const auto &SuperReg : RC->getMembers()) {
+ SubRegs.erase(SuperReg);
+ for (const auto &SubRegAndIdxOfSuperReg : SuperReg->getSubRegs())
+ SubRegs.erase(SubRegAndIdxOfSuperReg.second);
+ }
+
+ if (SubRegs.empty()) {
+ std::string TmpKind2 = (Twine(TmpKind) + " " +
+ " every register is covered by " + RC->getName())
+ .str();
+ VisitFn(&PossibleSubclass, TmpKind2);
+ } else {
+ DEBUG(dbgs() << "Rejected " << PossibleSubclass.getName()
+ << ", these registers are not covered by " << RC->getName()
+ << ":\n");
+ for (const auto &Reg : SubRegs)
+ DEBUG(dbgs() << " " << Reg->getName() << "\n");
}
}
}
@@ -286,11 +299,13 @@
for (const CodeGenRegisterClass *RC :
Bank.getExplictlySpecifiedRegisterClasses(RegisterClassHierarchy)) {
visitRegisterBankClasses(
- RegisterClassHierarchy, RC, "explicit",
+ RegisterClassHierarchy, RC,
+ "because " + RC->getName() + " was explicitly-added",
[&Bank](const CodeGenRegisterClass *RC, StringRef Kind) {
- DEBUG(dbgs() << "Added " << RC->getName() << "(" << Kind << ")\n");
+ DEBUG(dbgs() << "Added " << RC->getName() << " (" << Kind << ")\n");
Bank.addRegisterClass(RC);
- }, VisitedRCs);
+ },
+ VisitedRCs);
}
Banks.push_back(Bank);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33352.99541.patch
Type: text/x-patch
Size: 3654 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170519/91d0e949/attachment.bin>
More information about the llvm-commits
mailing list