[PATCH] D27338: [globalisel] Tablegen-erate current Register Bank Information

Ahmed Bougacha via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 9 14:36:54 PST 2016


ab added a comment.

This seems reasonable, but:

- I don't think the PartialMapping exclusion mechanism is necessary.  What about adding the missing mappings manually before landing this patch?  Alternatively, replace the manual code with a copy of the generated mappings (once you have those) before any tablegen changes?  That lets us split the generalization from the NFC tablegen changes without requiring hacks in-between.
- I'm not sure it's useful to generate the asserts: I think the intent was to check 1) the correctness of the manual description, and 2) it being consistent with the eventual tablegen output. Have you considered keeping them in AArch64RegisterBankInfo?  I suppose generating them lets all targets have them. @qcolombet: what do you think?
- Also, can you extract the RegisterBank code to a shared location? (CodeGenRegBank? maybe rename it to CodeGenRegisters) ?  That'll let us reuse it from the selector emitter backend.  We can do that separately though.



================
Comment at: utils/TableGen/RegisterBankEmitter.cpp:343
+void RegisterBankEmitter::run(raw_ostream &OS) {
+  std::vector<Record*> Targets = Records.getAllDerivedDefinitions("Target");
+  if (Targets.size() != 1)
----------------
<Record *>


================
Comment at: utils/TableGen/RegisterBankEmitter.cpp:362
+
+  OS << "// This file is generated. Do not edit.\n\n";
+  emitHeader(OS, TargetName, Banks);
----------------
emitSourceFileHeader ?


https://reviews.llvm.org/D27338





More information about the llvm-commits mailing list