[PATCH] D70501: [SystemZ] Don't build a PPA instruction with an immediate 0

Mon Nov 25 09:14:19 PST 2019

uweigand added a comment.

In D70501#1758659 <https://reviews.llvm.org/D70501#1758659>, @jonpa wrote:

> I thought that since generally a NoRegister could be the result of a bug somewhere in the compiler, it would be best to handle it as a special case only where it is expected. If that's not useful, I think it could be handled generally in SystemZInstPrinter.cpp, to just print '0' for any such operand. Seems like it would have to be handled both in SystemZInstPrinter::printOperand() and SystemZInstPrinter::printRegName(), but I'm not sure.

Right now I don't think this crash is a reliable check anyway -- e.g. if you directly emit machine code (not assembler text), you won't get a crash either.

So I think it should be OK to just have printOperand() accept NoRegister and print "0".   It looks like printRegName() does not need to handle NoRegister; this is used by common code to print register names for CFI directives and the like, where we should never have NoRegister.

(Another interesting question is whether our AsmParser will actually correctly read back and accept "0" in a register slot ... we should probably test this as well.)

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

https://reviews.llvm.org/D70501