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

[Alexander Richardson via Phabricator via llvm-commits]

arichardson added a comment.

In D110143#3012500 <https://reviews.llvm.org/D110143#3012500>, @sebastian-ne wrote:

> Good idea, I added a comment.
>
>> All the other update scripts seem to use `invoke_tool()` so we should be able to drop %s handling from those scripts too? I guess substitution twice doesn't do any harm but it would be good to remove duplication to make future refactoring easier.
>
> %s is not substituted in the scripts (except update_cc_test_checks.py), it is just removed with `.replace('< %s', '').replace('%s', '')`. We can move that to invoke_tool() if you think it makes sense?
>
>> Then it should be possible to use %s-O3.s in the tests instead of %S?
>
> I tried exactly that before, but for the above reason, that %s is removed and not substituted, this doesn’t work (it tries to run `cat -03.s`).

Does the following work:

  # RUN: cp -f %S/Inputs/amdgpu_no_merge_comments-O0.s %t-O0.s && cp -f %S/Inputs/amdgpu_no_merge_comments-O3.s %t-O3.s
  # 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

And then inside the actual test

  ; llc is replaced with cat, we just simulate llc by printing text from these files
  ; RUN: llc %S/amdgpu_no_merge_comments.tmp-O0.s | FileCheck -check-prefixes=GCN,GFX9-O0 %s
  ; RUN: llc %S/amdgpu_no_merge_comments.tmp-O3.s | FileCheck -check-prefixes=GCN,GFX9-O3 %s

Not sure if that's much better than depending on %T though.



================
Comment at: llvm/utils/UpdateTestChecks/common.py:172
       assert isinstance(preprocess_cmd, str)  # TODO: use a list instead of using shell
       preprocess_cmd = preprocess_cmd.replace('%s', ir).strip()
       if verbose:
----------------
Just noticed that we should also be using `applySubstitutions` here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110143