[PATCH] D116463: [SPIRV 4/6] Add target lowerling, TargetMachine and AsmPrinter

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp:64
+
+  if (isVerbose())
+    OutStreamer->GetCommentOS()
----------------
Braces


================
Comment at: llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp:31
+                                    Register SwiftErrorVReg) const {
+  assert(VRegs.size() < 2 && "All return types should use a single register");
+  if (Val) {
----------------
You can't guarantee this. A return of struct with multiple fields will hit it


================
Comment at: llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp:56
+    for (const auto &Arg : F.args()) {
+      assert(VRegs[i].size() == 1 && "Formal arg has multiple vregs");
+      ArgTypeVRegs.push_back(
----------------
This will happen for any struct arguments


================
Comment at: llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp:89
+                                  CallLoweringInfo &Info) const {
+  assert(Info.OrigRet.Regs.size() < 2 && "Call returns multiple vregs");
+
----------------
Struct returns


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

https://reviews.llvm.org/D116463