[PATCH] D116465: [SPIRV 6/6] Add 2 essential passes and the simplest tests

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

You don't seem to have tests checking if the encoding of the instructions are correct when printing or parsing, leaving the Printer/Parser completely untested.

I'm guessing you don't have any arguments or return values because you haven't yet implemented prologue/epilogue and frame lowering.

Without that, it's hard to really test anything, but also makes the two passes a bit pointless, because you're not testing them either.



================
Comment at: llvm/lib/Target/SPIRV/SPIRVBlockLabeler.cpp:11
+// start with an OpLabel, and ends with a suitable terminator (OpBranch is
+// inserted if necessary).
+//
----------------
Isn't this guaranteed by IR passes and the verifier?

What would create basic blocks that don't have a suitable terminator that still passes the verify?


================
Comment at: llvm/lib/Target/SPIRV/SPIRVGlobalTypesAndRegNumPass.cpp:23
+// that are technically invalid as a result. As such, it must be the last pass,
+// and requires instruction verification to be disabled afterwards.
+//
----------------
That's a bit scary. Is that what created BBs without terminators?


================
Comment at: llvm/lib/Target/SPIRV/SPIRVTargetMachine.cpp:158
+// Add custom passes right before emitting asm/obj files.
+void SPIRVPassConfig::addPreEmitPass2() {
+  // Insert missing block labels and terminators. Fix instrs with MBB references
----------------
I don't see where this is called in this code...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116465