This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Pretty-print CR bits the way the binutils disassembler does
ClosedPublic

Authored by nemanjai on Mar 30 2017, 4:49 AM.

Details

Summary

This is a small patch that allows the PPC back end to print these registers in a more human-readable form the way the binutils tools print them.

I'm not sure if anyone uses this option much, but it's nice to have when debugging.

Diff Detail

Repository
rL LLVM

Event Timeline

nemanjai created this revision.Mar 30 2017, 4:49 AM
hfinkel edited edge metadata.Mar 30 2017, 11:03 AM

This seems like a good thing to do.

lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp
452

This ShowVSRNumsAsVR seems broken ; should we just get rid of it?

454

Hrmm. Is TRI available here? The real way of doing this is to get the encoding value for the register. From PPCAsmPrinter.cpp, for example, we have this:

unsigned Mask = 0x80 >> OutContext.getRegisterInfo()
                        ->getEncodingValue(MI->getOperand(0).getReg());

It is possible that just using MRI.getEncodingValue(RegNum) will work here. It looks like it might (MCInstPrinter has an MRI protected member, and because the encoding values come from TableGen, I think that MCRegisterInfo should have them).

echristo edited edge metadata.Apr 11 2017, 1:46 PM

Were we going to pick this up again? Still seems useful.

echristo requested changes to this revision.Apr 11 2017, 1:46 PM
This revision now requires changes to proceed.Apr 11 2017, 1:46 PM
nemanjai added inline comments.May 28 2017, 10:14 PM
lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp
452

Can you elaborate a bit on why you feel it's broken? It should just display things like vs34 as v2 and we use it here just to prevent the v from getting stripped from the output.

454

Ah, thanks for the pointer. This makes perfect sense.

nemanjai updated this revision to Diff 100578.May 28 2017, 10:15 PM
nemanjai edited edge metadata.

Updated to use the actual register encoding that comes from the MRI rather than converting the string representation.

hfinkel added inline comments.May 29 2017, 1:33 AM
lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp
452

I should have been more specific. I meant that it seems incorrect that ShowVSRNumsAsVR is equivalent to FullRegNames here (it is affecting much more than just the vector registers).

nemanjai marked an inline comment as done.May 29 2017, 12:58 PM
nemanjai added inline comments.
lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp
452

Ah, that makes sense. Basically get rid of the semantics where ShowVSRNumsAsVR implies FullRegNames.

nemanjai updated this revision to Diff 100635.May 29 2017, 12:59 PM
nemanjai marked an inline comment as done.

Removed the "show vsr nums as vr" implies "print full register names" semantics.

echristo accepted this revision.Jun 29 2017, 12:26 AM

LGTM.

-eric

This revision is now accepted and ready to land.Jun 29 2017, 12:26 AM
This revision was automatically updated to reflect the committed changes.