[PATCH] D70660: Add initial tests for update_{llc,cc}_test_checks.py

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Dec 8 22:17:23 PST 2019


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

LGTM with two comments. (You can commit with the appropriate changes.)

In D70660#1765563 <https://reviews.llvm.org/D70660#1765563>, @arichardson wrote:

> In D70660#1765478 <https://reviews.llvm.org/D70660#1765478>, @jdoerfert wrote:
>
> > - We still have the "inputs" folder in which everything lives. I could see the '.ll' and '.ll.expected' to live in their but why the tests?
>
>
> Only the .ll and .ll.expected is in the Inputs folder. The .test shouldn't be.




1. I mean that is what I want but the current version looks different.

> I am happy to move to a separate .expected



2. Please do.



================
Comment at: llvm/test/tools/UpdateTestChecks/update_test_checks/basic.test:44
+# FUNCSIG-NEXT:  +; CHECK-SAME: (<2 x i8> {{\[}}[X:%.*]], <2 x i8> {{\[}}[Y:%.*]])
+# FUNCSIG-NEXT:   ; CHECK-NEXT:    ret <2 x i8> zeroinitializer
----------------
arichardson wrote:
> jdoerfert wrote:
> > Shouldn't we have multiple .expected files instead? I see myself struggle with the escaping already.
> Since these files are quite small I guess that would be fine. I agree that the escaping is ugly.
> 
> The only benefit of having the manual check lines is that it shows the difference between the two files but that can also be seen by comparing the `<test>.expected` with a `<test>.funcsig.expected`
I'd prefer a new file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70660





More information about the llvm-commits mailing list