[CodeGen] Don't print register classes in -debug output

Description

[CodeGen] Don't print register classes in -debug output

Since register classes and banks are already printed with the register
definition, don't print it at the end of every instruction anymore.

This follows MIR in this regard and is another step to the unification
of the two formats.

Details

Committed
thegamegJan 9 2018, 7:39 AM
Parents
rL322085: [DAG] Elide overlapping stores
Branches
Unknown
Tags
Unknown
MatzeB added a subscriber: MatzeB.Jan 16 2018, 9:22 AM

Hmm this change is not equivalent. Take a random example of a testcase here:

%2:gpr = SMULBB %1, %1, pred:14, pred:%noreg; GPR:%2,%1,%1

now becomes:

%2:gpr = SMULBB %1, %1, pred:14, pred:%noreg

the register class of %1 is no longer visible.

While it is fine to only print the regclass for definitions when printing a whole .mir file, when we only print a single instruction we need the register class on use operands as well!

Hmm this change is not equivalent. Take a random example of a testcase here:

%2:gpr = SMULBB %1, %1, pred:14, pred:%noreg; GPR:%2,%1,%1

now becomes:

%2:gpr = SMULBB %1, %1, pred:14, pred:%noreg

the register class of %1 is no longer visible.

While it is fine to only print the regclass for definitions when printing a whole .mir file, when we only print a single instruction we need the register class on use operands as well!

I agree. Although I would like to get rid of the "; GPR..." syntax.

How about:

%2:gpr = SMULBB %1:gpr, %1:gpr, 14, %noreg

This would allow us to still have valid MIR.

I agree, we need the classes when printing single/few instructions. I also think this generally makes it harder to debug things which is contradictory to the purpose of -debug.

This follows MIR in this regard and is another step to the unification
of the two formats.

MIR will accept them on the uses too so long as the uses are consistent with the def. It doesn't normally print them again, but now that it's been mentioned we may want to reconsider that in some contexts like -debug. For example, in the instruction selector's debug we see things like:

Selecting: 
  %2:gpr(s32) = G_ICMP intpred(ne), %0(s64), %1

This isn't actually all the information that the instruction selector will consider so it's not completely useful by itself. Something like this:

%2:gpr(s32) = G_ICMP intpred(ne), %0:gpr(s64), %1:gpr(s64)

would be fully self-contained so we don't need to refer to the full MIR dump to figure out what the instruction selector was supposed to match.

Hmm this change is not equivalent. Take a random example of a testcase here:

%2:gpr = SMULBB %1, %1, pred:14, pred:%noreg; GPR:%2,%1,%1

now becomes:

%2:gpr = SMULBB %1, %1, pred:14, pred:%noreg

the register class of %1 is no longer visible.

While it is fine to only print the regclass for definitions when printing a whole .mir file, when we only print a single instruction we need the register class on use operands as well!

I agree. Although I would like to get rid of the "; GPR..." syntax.

How about:

%2:gpr = SMULBB %1:gpr, %1:gpr, 14, %noreg

This would allow us to still have valid MIR.

Moving to that kind of output would resolve my concerns.

Hmm this change is not equivalent. Take a random example of a testcase here:

%2:gpr = SMULBB %1, %1, pred:14, pred:%noreg; GPR:%2,%1,%1

now becomes:

%2:gpr = SMULBB %1, %1, pred:14, pred:%noreg

the register class of %1 is no longer visible.

While it is fine to only print the regclass for definitions when printing a whole .mir file, when we only print a single instruction we need the register class on use operands as well!

I agree. Although I would like to get rid of the "; GPR..." syntax.

How about:

%2:gpr = SMULBB %1:gpr, %1:gpr, 14, %noreg

This would allow us to still have valid MIR.

Hmm this change is not equivalent. Take a random example of a testcase here:

%2:gpr = SMULBB %1, %1, pred:14, pred:%noreg; GPR:%2,%1,%1

now becomes:

%2:gpr = SMULBB %1, %1, pred:14, pred:%noreg

the register class of %1 is no longer visible.

While it is fine to only print the regclass for definitions when printing a whole .mir file, when we only print a single instruction we need the register class on use operands as well!

I agree. Although I would like to get rid of the "; GPR..." syntax.

How about:

%2:gpr = SMULBB %1:gpr, %1:gpr, 14, %noreg

This would allow us to still have valid MIR.

Yes that is what I was hoping for in the case of printing a single instruction or a single basic block. (But when printing a whole function we don't need the regclass on the uses as we should pretty much always have a def of the vreg somwhere).