[PATCH] D110143: [Utils] Replace llc with cat for tests

Tue Sep 21 05:14:22 PDT 2021

sebastian-ne added inline comments.

================
Comment at: llvm/test/tools/UpdateTestChecks/update_llc_test_checks/amdgpu-no-merge-comments.test:5
+# RUN: cp -f %S/Inputs/amdgpu_no_merge_comments-O0.s %T/ && cp -f %S/Inputs/amdgpu_no_merge_comments-O3.s %T/
+# RUN: cp -f %S/Inputs/amdgpu_no_merge_comments.ll %t.ll && %update_llc_test_checks --llc-binary cat %t.ll
 # RUN: diff -u %S/Inputs/amdgpu_no_merge_comments.ll.expected %t.ll
----------------
arichardson wrote:
> IMO this should be in a separate test file since it's not checking "that functions with different IR comments in the output are not merged" but instead is checking the %S lit substitution
Thanks for your review!

This is still testing "that functions with different IR comments in the output are not merged" with the `%update_llc_test_checks --llc-binary cat %t.ll` command.
The %S substitution in this file was working before (it is already used before this patch).
The two copies of the .s files are needed because %update_llc_test_checks is run on a temporary file in `build/test/tools/UpdateTestChecks/update_llc_test_checks/Output/` and the tested .ll file `amdgpu_no_merge_comments.ll` expects the two .s files to be in the same directory.

The added `%S` support is only for running %update_llc_test_checks. The two RUN lines in `amdgpu_no_merge_comments.ll` are exercising this code (although not exactly for testing, but through necessity to somehow reference the .s files).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110143/new/

https://reviews.llvm.org/D110143