[PATCH] D42675: [RISCV] Define getSetCCResultType for setting vector setCC type

[Alex Bradbury via Phabricator via llvm-commits]

asb accepted this revision.
asb added a comment.
This revision is now accepted and ready to land.

Hi Shiva. update_llc_test_checks.py basically generates CHECK lines for you according to the RUN lines in the file. This is a fairly minor benefit when writing a test for the first time, but the biggest benefit comes from maintaining the test. When there is a codegen change, test files that contain all instructions are much easier to review.

Previous workflow:

- Create and commit a test, manually write CHECK lines, typically picking out a subset of instructions to CHECK
- A change is later made that affects codegen. Typically you would manually inspect the previous and new output, then manually update CHECK lines

New work flow:

- Create a test, have update_llc_test_checks.py generate CHECK lines for you. Double-check they are as expected before committing
- A change is later made that affects codegen. I normally immediately use update_llc_test_checks.py to update a file, then use git diff to review the change for correctness.
- As an example, the use of update_llc_test_checks.py made it totally trivial to update all codegen tests when @niosHD introduced pseudoinstruction aliases

The risk of update_llc_test_checks.py is that by checking each instruction, tests are more brittle to codegen changes. I haven't found this to be an issue in practice, and flagging up even minor codegen differences can be an advantage. An increasing number of tests for other backends (especially x86) are moving over to doing this for the maintainability benefits.

Assuming your LLVM build dir is within your LLVM checkout (e.g. `build`), you can invoke update_llc_test_checks.py like: `../utils/update_llc_test_checks.py --llc-binary=./bin/llc ../test/CodeGen/RISCV/get-setcc-result-type.ll`.

This looks good to commit to me, once the two trivial comments I've added on the test are addressed. Thanks!



================
Comment at: test/CodeGen/RISCV/get-setcc-result-type.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -disable-block-placement -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV32I %s
----------------
`-disable-block-placement` has no codegen impact for this test, so remove the option.


================
Comment at: test/CodeGen/RISCV/get-setcc-result-type.ll:7-28
+; RV32I:       # %bb.0:
+; RV32I-NEXT: lw	a1, 12(a0)
+; RV32I-NEXT: xor	a1, a1, zero
+; RV32I-NEXT: seqz	a1, a1
+; RV32I-NEXT: neg	a1, a1
+; RV32I-NEXT: sw	a1, 12(a0)
+; RV32I-NEXT: lw	a1, 8(a0)
----------------
Whitespace here doesn't match the output of `update_llc_test_checks.py`, meaning we'll get unwanted diffs the next time the test is regenerated. Use the invocation I explain in my other comment to regenerate.


Repository:
  rL LLVM

https://reviews.llvm.org/D42675