Skip to content

[TableGen][NFC] Use early exit to simplify large block in emitAction. #138220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 8, 2025

Conversation

nvjle
Copy link
Contributor

@nvjle nvjle commented May 2, 2025

Most of the processing in emitAction is in an unneeded else-block-- reduce indentation by exiting after the recursive call.

XXXGenCallingConv.inc are identical before and after this patch for all targets.

Most of the processing in emitAction is in an unneeded else-block--
reduce indentation by exiting after the recursive call.

Also use ranged-for in one instance.

`XXXGenCallingConv.inc` are identical before and after this patch for
all targets.
@nvjle nvjle requested review from mshockwave and topperc May 2, 2025 00:01
@llvmbot
Copy link
Member

llvmbot commented May 2, 2025

@llvm/pr-subscribers-tablegen

Author: Jason Eckhardt (nvjle)

Changes

Most of the processing in emitAction is in an unneeded else-block-- reduce indentation by exiting after the recursive call.

Also use ranged-for in one instance.

XXXGenCallingConv.inc are identical before and after this patch for all targets.


Full diff: https://github.com/llvm/llvm-project/pull/138220.diff

1 Files Affected:

  • (modified) llvm/utils/TableGen/CallingConvEmitter.cpp (+115-118)
diff --git a/llvm/utils/TableGen/CallingConvEmitter.cpp b/llvm/utils/TableGen/CallingConvEmitter.cpp
index 20dc1ff0ffd14..75d292076fd3c 100644
--- a/llvm/utils/TableGen/CallingConvEmitter.cpp
+++ b/llvm/utils/TableGen/CallingConvEmitter.cpp
@@ -204,13 +204,11 @@ void CallingConvEmitter::emitAction(const Record *Action, indent Indent,
 
     if (Action->isSubClassOf("CCIfType")) {
       const ListInit *VTs = Action->getValueAsListInit("VTs");
-      for (unsigned I = 0, E = VTs->size(); I != E; ++I) {
-        const Record *VT = VTs->getElementAsRecord(I);
-        if (I != 0)
-          O << " ||\n    " << Indent;
-        O << "LocVT == " << getEnumName(getValueType(VT));
+      ListSeparator LS(" ||\n    " + std::string(Indent.NumIndents, ' '));
+      for (const Init *I : VTs->getValues()) {
+        const Record *VT = cast<DefInit>(I)->getDef();
+        O << LS << "LocVT == " << getEnumName(getValueType(VT));
       }
-
     } else if (Action->isSubClassOf("CCIf")) {
       O << Action->getValueAsString("Predicate");
     } else {
@@ -221,120 +219,119 @@ void CallingConvEmitter::emitAction(const Record *Action, indent Indent,
     O << ") {\n";
     emitAction(Action->getValueAsDef("SubAction"), Indent + 2, O);
     O << Indent << "}\n";
-  } else {
-    if (Action->isSubClassOf("CCDelegateTo")) {
-      const Record *CC = Action->getValueAsDef("CC");
-      O << Indent << "if (!" << CC->getName()
-        << "(ValNo, ValVT, LocVT, LocInfo, ArgFlags, State))\n"
-        << Indent + 2 << "return false;\n";
-      DelegateToMap[CurrentAction].insert(CC->getName().str());
-    } else if (Action->isSubClassOf("CCAssignToReg") ||
-               Action->isSubClassOf("CCAssignToRegTuple") ||
-               Action->isSubClassOf("CCAssignToRegAndStack")) {
-      const ListInit *RegList = Action->getValueAsListInit("RegList");
-      for (unsigned I = 0, E = RegList->size(); I != E; ++I) {
-        std::string Name = getQualifiedRegisterName(RegList->getElement(I));
-        if (SwiftAction)
-          AssignedSwiftRegsMap[CurrentAction].insert(std::move(Name));
-        else
-          AssignedRegsMap[CurrentAction].insert(std::move(Name));
-      }
-      EmitAllocateReg({RegList}, {"RegList"});
-
-      if (Action->isSubClassOf("CCAssignToRegAndStack"))
-        EmitAllocateStack();
-
-      O << Indent << "  return false;\n";
-      O << Indent << "}\n";
-    } else if (Action->isSubClassOf("CCAssignToRegWithShadow")) {
-      const ListInit *RegList = Action->getValueAsListInit("RegList");
-      const ListInit *ShadowRegList =
-          Action->getValueAsListInit("ShadowRegList");
-      if (!ShadowRegList->empty() && ShadowRegList->size() != RegList->size())
-        PrintFatalError(Action->getLoc(),
-                        "Invalid length of list of shadowed registers");
-
-      EmitAllocateReg({RegList, ShadowRegList}, {"RegList", "RegList"});
-
-      O << Indent << "  return false;\n";
-      O << Indent << "}\n";
-    } else if (Action->isSubClassOf("CCAssignToStack")) {
-      EmitAllocateStack(/*EmitOffset=*/true);
-      O << Indent << "State.addLoc(CCValAssign::getMem(ValNo, ValVT, Offset"
-        << Counter << ", LocVT, LocInfo));\n";
-      O << Indent << "return false;\n";
-    } else if (Action->isSubClassOf("CCAssignToStackWithShadow")) {
-      int Size = Action->getValueAsInt("Size");
-      int Align = Action->getValueAsInt("Align");
-      const ListInit *ShadowRegList =
-          Action->getValueAsListInit("ShadowRegList");
-
-      unsigned ShadowRegListNumber = ++Counter;
-      EmitRegList(ShadowRegList, "ShadowRegList" + utostr(ShadowRegListNumber));
-
-      O << Indent << "int64_t Offset" << ++Counter << " = State.AllocateStack("
-        << Size << ", Align(" << Align << "), "
-        << "ShadowRegList" << ShadowRegListNumber << ");\n";
-      O << Indent << "State.addLoc(CCValAssign::getMem(ValNo, ValVT, Offset"
-        << Counter << ", LocVT, LocInfo));\n";
-      O << Indent << "return false;\n";
-    } else if (Action->isSubClassOf("CCPromoteToType")) {
-      const Record *DestTy = Action->getValueAsDef("DestTy");
-      MVT::SimpleValueType DestVT = getValueType(DestTy);
-      O << Indent << "LocVT = " << getEnumName(DestVT) << ";\n";
-      if (MVT(DestVT).isFloatingPoint()) {
-        O << Indent << "LocInfo = CCValAssign::FPExt;\n";
-      } else {
-        O << Indent << "if (ArgFlags.isSExt())\n"
-          << Indent << "  LocInfo = CCValAssign::SExt;\n"
-          << Indent << "else if (ArgFlags.isZExt())\n"
-          << Indent << "  LocInfo = CCValAssign::ZExt;\n"
-          << Indent << "else\n"
-          << Indent << "  LocInfo = CCValAssign::AExt;\n";
-      }
-    } else if (Action->isSubClassOf("CCPromoteToUpperBitsInType")) {
-      const Record *DestTy = Action->getValueAsDef("DestTy");
-      MVT::SimpleValueType DestVT = getValueType(DestTy);
-      O << Indent << "LocVT = " << getEnumName(DestVT) << ";\n";
-      if (MVT(DestVT).isFloatingPoint()) {
-        PrintFatalError(Action->getLoc(),
-                        "CCPromoteToUpperBitsInType does not handle floating "
-                        "point");
-      } else {
-        O << Indent << "if (ArgFlags.isSExt())\n"
-          << Indent << "  LocInfo = CCValAssign::SExtUpper;\n"
-          << Indent << "else if (ArgFlags.isZExt())\n"
-          << Indent << "  LocInfo = CCValAssign::ZExtUpper;\n"
-          << Indent << "else\n"
-          << Indent << "  LocInfo = CCValAssign::AExtUpper;\n";
-      }
-    } else if (Action->isSubClassOf("CCBitConvertToType")) {
-      const Record *DestTy = Action->getValueAsDef("DestTy");
-      O << Indent << "LocVT = " << getEnumName(getValueType(DestTy)) << ";\n";
-      O << Indent << "LocInfo = CCValAssign::BCvt;\n";
-    } else if (Action->isSubClassOf("CCTruncToType")) {
-      const Record *DestTy = Action->getValueAsDef("DestTy");
-      O << Indent << "LocVT = " << getEnumName(getValueType(DestTy)) << ";\n";
-      O << Indent << "LocInfo = CCValAssign::Trunc;\n";
-    } else if (Action->isSubClassOf("CCPassIndirect")) {
-      const Record *DestTy = Action->getValueAsDef("DestTy");
-      O << Indent << "LocVT = " << getEnumName(getValueType(DestTy)) << ";\n";
-      O << Indent << "LocInfo = CCValAssign::Indirect;\n";
-    } else if (Action->isSubClassOf("CCPassByVal")) {
-      int Size = Action->getValueAsInt("Size");
-      int Align = Action->getValueAsInt("Align");
-      O << Indent << "State.HandleByVal(ValNo, ValVT, LocVT, LocInfo, " << Size
-        << ", Align(" << Align << "), ArgFlags);\n";
-      O << Indent << "return false;\n";
-    } else if (Action->isSubClassOf("CCCustom")) {
-      O << Indent << "if (" << Action->getValueAsString("FuncName")
-        << "(ValNo, ValVT, "
-        << "LocVT, LocInfo, ArgFlags, State))\n";
-      O << Indent << "  return false;\n";
+    return;
+  }
+
+  if (Action->isSubClassOf("CCDelegateTo")) {
+    const Record *CC = Action->getValueAsDef("CC");
+    O << Indent << "if (!" << CC->getName()
+      << "(ValNo, ValVT, LocVT, LocInfo, ArgFlags, State))\n"
+      << Indent + 2 << "return false;\n";
+    DelegateToMap[CurrentAction].insert(CC->getName().str());
+  } else if (Action->isSubClassOf("CCAssignToReg") ||
+             Action->isSubClassOf("CCAssignToRegTuple") ||
+             Action->isSubClassOf("CCAssignToRegAndStack")) {
+    const ListInit *RegList = Action->getValueAsListInit("RegList");
+    for (unsigned I = 0, E = RegList->size(); I != E; ++I) {
+      std::string Name = getQualifiedRegisterName(RegList->getElement(I));
+      if (SwiftAction)
+        AssignedSwiftRegsMap[CurrentAction].insert(std::move(Name));
+      else
+        AssignedRegsMap[CurrentAction].insert(std::move(Name));
+    }
+    EmitAllocateReg({RegList}, {"RegList"});
+
+    if (Action->isSubClassOf("CCAssignToRegAndStack"))
+      EmitAllocateStack();
+
+    O << Indent << "  return false;\n";
+    O << Indent << "}\n";
+  } else if (Action->isSubClassOf("CCAssignToRegWithShadow")) {
+    const ListInit *RegList = Action->getValueAsListInit("RegList");
+    const ListInit *ShadowRegList = Action->getValueAsListInit("ShadowRegList");
+    if (!ShadowRegList->empty() && ShadowRegList->size() != RegList->size())
+      PrintFatalError(Action->getLoc(),
+                      "Invalid length of list of shadowed registers");
+
+    EmitAllocateReg({RegList, ShadowRegList}, {"RegList", "RegList"});
+
+    O << Indent << "  return false;\n";
+    O << Indent << "}\n";
+  } else if (Action->isSubClassOf("CCAssignToStack")) {
+    EmitAllocateStack(/*EmitOffset=*/true);
+    O << Indent << "State.addLoc(CCValAssign::getMem(ValNo, ValVT, Offset"
+      << Counter << ", LocVT, LocInfo));\n";
+    O << Indent << "return false;\n";
+  } else if (Action->isSubClassOf("CCAssignToStackWithShadow")) {
+    int Size = Action->getValueAsInt("Size");
+    int Align = Action->getValueAsInt("Align");
+    const ListInit *ShadowRegList = Action->getValueAsListInit("ShadowRegList");
+
+    unsigned ShadowRegListNumber = ++Counter;
+    EmitRegList(ShadowRegList, "ShadowRegList" + utostr(ShadowRegListNumber));
+
+    O << Indent << "int64_t Offset" << ++Counter << " = State.AllocateStack("
+      << Size << ", Align(" << Align << "), "
+      << "ShadowRegList" << ShadowRegListNumber << ");\n";
+    O << Indent << "State.addLoc(CCValAssign::getMem(ValNo, ValVT, Offset"
+      << Counter << ", LocVT, LocInfo));\n";
+    O << Indent << "return false;\n";
+  } else if (Action->isSubClassOf("CCPromoteToType")) {
+    const Record *DestTy = Action->getValueAsDef("DestTy");
+    MVT::SimpleValueType DestVT = getValueType(DestTy);
+    O << Indent << "LocVT = " << getEnumName(DestVT) << ";\n";
+    if (MVT(DestVT).isFloatingPoint()) {
+      O << Indent << "LocInfo = CCValAssign::FPExt;\n";
     } else {
-      errs() << *Action;
-      PrintFatalError(Action->getLoc(), "Unknown CCAction!");
+      O << Indent << "if (ArgFlags.isSExt())\n"
+        << Indent << "  LocInfo = CCValAssign::SExt;\n"
+        << Indent << "else if (ArgFlags.isZExt())\n"
+        << Indent << "  LocInfo = CCValAssign::ZExt;\n"
+        << Indent << "else\n"
+        << Indent << "  LocInfo = CCValAssign::AExt;\n";
+    }
+  } else if (Action->isSubClassOf("CCPromoteToUpperBitsInType")) {
+    const Record *DestTy = Action->getValueAsDef("DestTy");
+    MVT::SimpleValueType DestVT = getValueType(DestTy);
+    O << Indent << "LocVT = " << getEnumName(DestVT) << ";\n";
+    if (MVT(DestVT).isFloatingPoint()) {
+      PrintFatalError(Action->getLoc(),
+                      "CCPromoteToUpperBitsInType does not handle floating "
+                      "point");
+    } else {
+      O << Indent << "if (ArgFlags.isSExt())\n"
+        << Indent << "  LocInfo = CCValAssign::SExtUpper;\n"
+        << Indent << "else if (ArgFlags.isZExt())\n"
+        << Indent << "  LocInfo = CCValAssign::ZExtUpper;\n"
+        << Indent << "else\n"
+        << Indent << "  LocInfo = CCValAssign::AExtUpper;\n";
     }
+  } else if (Action->isSubClassOf("CCBitConvertToType")) {
+    const Record *DestTy = Action->getValueAsDef("DestTy");
+    O << Indent << "LocVT = " << getEnumName(getValueType(DestTy)) << ";\n";
+    O << Indent << "LocInfo = CCValAssign::BCvt;\n";
+  } else if (Action->isSubClassOf("CCTruncToType")) {
+    const Record *DestTy = Action->getValueAsDef("DestTy");
+    O << Indent << "LocVT = " << getEnumName(getValueType(DestTy)) << ";\n";
+    O << Indent << "LocInfo = CCValAssign::Trunc;\n";
+  } else if (Action->isSubClassOf("CCPassIndirect")) {
+    const Record *DestTy = Action->getValueAsDef("DestTy");
+    O << Indent << "LocVT = " << getEnumName(getValueType(DestTy)) << ";\n";
+    O << Indent << "LocInfo = CCValAssign::Indirect;\n";
+  } else if (Action->isSubClassOf("CCPassByVal")) {
+    int Size = Action->getValueAsInt("Size");
+    int Align = Action->getValueAsInt("Align");
+    O << Indent << "State.HandleByVal(ValNo, ValVT, LocVT, LocInfo, " << Size
+      << ", Align(" << Align << "), ArgFlags);\n";
+    O << Indent << "return false;\n";
+  } else if (Action->isSubClassOf("CCCustom")) {
+    O << Indent << "if (" << Action->getValueAsString("FuncName")
+      << "(ValNo, ValVT, "
+      << "LocVT, LocInfo, ArgFlags, State))\n";
+    O << Indent << "  return false;\n";
+  } else {
+    errs() << *Action;
+    PrintFatalError(Action->getLoc(), "Unknown CCAction!");
   }
 }
 

nvjle added 3 commits May 2, 2025 13:54
Causes output difference on Win. I don't have Windows locally to
debug/repro. So just leave the main part of the patch.
@nvjle
Copy link
Contributor Author

nvjle commented May 8, 2025

Gentle ping...

Copy link
Contributor

@jayfoad jayfoad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nvjle nvjle merged commit 9692dff into llvm:main May 8, 2025
11 checks passed
lenary added a commit to lenary/llvm-project that referenced this pull request May 9, 2025
* main: (420 commits)
  [AArch64] Merge scaled and unscaled narrow zero stores (llvm#136705)
  [RISCV] One last migration to getInsertSubvector [nfc]
  [flang][OpenMP] Update `do concurrent` mapping pass to use `fir.do_concurrent` op (llvm#138489)
  [MLIR][LLVM] Fix llvm.mlir.global mismatching print and parser order (llvm#138986)
  [lld][NFC] Fix minor typo in docs (llvm#138898)
  [RISCV] Migrate getConstant indexed insert/extract subvector to new API (llvm#139111)
  GlobalISel: Translate minimumnum and maximumnum (llvm#139106)
  [MemProf] Simplify unittest save and restore of options (llvm#139117)
  [BOLT][AArch64] Patch functions targeted by optional relocs (llvm#138750)
  [Coverage] Support -fprofile-list for cold function coverage (llvm#136333)
  Remove unused forward decl (llvm#139108)
  [AMDGPU][NFC] Get rid of OPW constants. (llvm#139074)
  [CIR] Upstream extract op for VectorType (llvm#138413)
  [mlir][xegpu] Handle scalar uniform ops in SIMT distribution.  (llvm#138593)
  [GlobalISel][AMDGPU] Fix handling of v2i128 type for AND, OR, XOR (llvm#138574)
  AMDGPU][True16][CodeGen] FP_Round f64 to f16 in true16 (llvm#128911)
  Reland [Clang] Deprecate `__is_trivially_relocatable` (llvm#139061)
  [HLSL][NFC] Stricter Overload Tests (clamp,max,min,pow) (llvm#138993)
  [MLIR] Fixing the memref linearization size computation for non-packed memref (llvm#138922)
  [TableGen][NFC] Use early exit to simplify large block in emitAction. (llvm#138220)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants