[PATCH] D43328: [RISCV] Update MC compression tests

Alex Bradbury via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 27 13:22:10 PST 2018


asb added a comment.
Herald added a subscriber: shiva0217.

If you explicitly add me as a reviewer for any RISC-V patches it's easier for me to see it on my TODO list. It looks like the stack of dependent patches is incomplete here - filling that out makes it a little easier to keep track of what's reviewed/committed etc.

Minor issue with compress-relocations.s, but otherwise seems sensible. I'd hold off committing until the compression patch is reviewed, primarily because suggestions to change testing in that patch might mean you'd want to come back and edit these changes.

The issue with fixups-compressed.s is a good spot, the best fix would be to update that so there's a FileCheck line that ensures no relocations are generated (similar to fixups.s). That fix could be committed directly, as that's a straight-forward improvement that makes sense regardless of any instruction compression patches.



================
Comment at: test/MC/RISCV/compress-relocations.s:1-13
+# RUN: llvm-mc -triple riscv32 -mattr=+c -riscv-no-aliases < %s -show-encoding \
+# RUN:     | FileCheck -check-prefix=INSTR -check-prefix=FIXUP %s
+
+# Check prefixes:
+# FIXUP - Check the fixup on the instruction.
+# INSTR - Check the instruction is handled properly by the ASMPrinter
+c.jal foo
----------------
You've lost the RELOC checks.


https://reviews.llvm.org/D43328





More information about the llvm-commits mailing list