[PATCH] D78318: [GlobalISel][InlineAsm] Add support for basic output operand constraints

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp:90
+
+/// GetRegistersForValue - Assign virtual/physical registers for the specified
+/// register operand.
----------------
Don't need to repeat function name


================
Comment at: llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp:92
+/// register operand.
+static void GetRegistersForValue(MachineFunction &MF,
+                                 MachineIRBuilder &MIRBuilder,
----------------
Start with lowercase


================
Comment at: llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp:119-120
+
+  // Remember the type of the register for later
+  OpInfo.RegType = *TRI.legalclasstypes_begin(*RC);
+
----------------
Picking types from classes is usually problematic for me. Do we really need this type, and if so, is it feasible to use LLT instead?


================
Comment at: llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp:140
+    for (; *I != AssignedReg; ++I)
+      assert(I != RC->end() && "AssignedReg should be member of RC");
+  }
----------------
RC->contains()?


================
Comment at: llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp:214
+
+static void ComputeConstraintToUse(const TargetLowering *TLI,
+                                   TargetLowering::AsmOperandInfo &OpInfo) {
----------------
Start with lowercase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78318/new/

https://reviews.llvm.org/D78318