This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Print human-readable VTYPE/SEW operands in MIR
ClosedPublic

Authored by frasercrmck on Apr 21 2022, 10:29 AM.

Details

Summary

This patch adds custom MIR operand comments to VTYPE immediate operands
in VSETVLI instructions and SEW operands in vector codegen pseudo
instructions. The result is intended to be more human-readable and
hopefully maintainable when working with MIR, particularly when
writing or reading test cases.

Diff Detail

Event Timeline

frasercrmck created this revision.Apr 21 2022, 10:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 21 2022, 10:29 AM
frasercrmck requested review of this revision.Apr 21 2022, 10:29 AM

I've had this patch kicking around locally for a long time and was never sure whether the extra verbosity would be welcome. So I thought I'd throw it out there to see what people think.

llvm/test/CodeGen/RISCV/rvv/addi-scalable-offset.mir
43

I wrote this when vsetvli was still printing without spaces between tokens. Presumably this should follow suit and have a space, for consistency?

craig.topper added inline comments.Apr 21 2022, 11:18 AM
llvm/test/CodeGen/RISCV/rvv/addi-scalable-offset.mir
43

Is printing m1 redundant with the instruction name?

llvm/test/CodeGen/RISCV/rvv/emergency-slot.mir
143–145

Was this test not updated when we converted from SEW to Log2SEW?

khchen added inline comments.Apr 21 2022, 8:00 PM
llvm/test/CodeGen/RISCV/rvv/emergency-slot.mir
91

Why the number is changed from 16 to 4?

craig.topper added inline comments.Apr 21 2022, 8:21 PM
llvm/test/CodeGen/RISCV/rvv/emergency-slot.mir
91

We used to use SEW instead of log2sew. This test wasn’t updated. 16 isn’t a valid log2sew value.

frasercrmck marked an inline comment as done.

rebase on test changes; drop lmul

frasercrmck marked an inline comment as done.Apr 22 2022, 1:00 AM
frasercrmck added inline comments.
llvm/test/CodeGen/RISCV/rvv/addi-scalable-offset.mir
43

Arguably, yeah... It's also not *really* part of the SEW operand and so might cause confusion when seeing how it's printed, so I've dropped it.

llvm/test/CodeGen/RISCV/rvv/emergency-slot.mir
143–145

Apparently not. I couldn't find any hints about what could have happened. I've pushed rG9687ca970f77 to fix this test up separately. I'm not sure why I didn't update the vslidedowns in this patch, though.

This revision is now accepted and ready to land.Apr 22 2022, 9:00 AM
frasercrmck retitled this revision from [RISCV] Print human-readable VTYPE/SEW/LMUL in MIR to [RISCV] Print human-readable VTYPE/SEW in MIR.Apr 22 2022, 9:25 AM
frasercrmck edited the summary of this revision. (Show Details)
frasercrmck retitled this revision from [RISCV] Print human-readable VTYPE/SEW in MIR to [RISCV] Print human-readable VTYPE/SEW operands in MIR.
This revision was landed with ongoing or failed builds.Apr 22 2022, 9:26 AM
This revision was automatically updated to reflect the committed changes.
frasercrmck marked an inline comment as done.

It's already accepted and committed, but I just want to say thanks, @frasercrmck, I always need to spend time to decode that when I read the value in MIR, that really save my life.

It's already accepted and committed, but I just want to say thanks, @frasercrmck, I always need to spend time to decode that when I read the value in MIR, that really save my life.

+1, it's really helpful, good job @frasercrmck!