Skip to content

[EH] Convert EH tests into lit tests - #3923

Merged
aheejin merged 2 commits into
WebAssembly:mainfrom
aheejin:eh_lit_tests
Jun 8, 2021
Merged

[EH] Convert EH tests into lit tests#3923
aheejin merged 2 commits into
WebAssembly:mainfrom
aheejin:eh_lit_tests

Conversation

@aheejin

@aheejin aheejin commented Jun 7, 2021

Copy link
Copy Markdown
Member

This converts most EH tests in test/passes into test/lit/passes. Fixed
some files to follow 2-space indentation and improved some comments.

This converts most EH tests in test/passes into test/lit/passes. Fixed
some files to follow 2-space indentation and improved some comments.
@aheejin
aheejin requested review from kripken and tlively June 7, 2021 17:08
@tlively

tlively commented Jun 7, 2021

Copy link
Copy Markdown
Member

Awesome! Besides the edits you mentioned, did you just use the update_lit_checks.py script to do this conversion?

@aheejin

aheejin commented Jun 7, 2021

Copy link
Copy Markdown
Member Author

Except for 2-3 tests yes, which required 1. stack IR printing or 2. module elements printing. They don't have the

;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.

at the top.

I thought about not autogenerating the whole function in case it is slightly longer but only adding lines that are relevant, but it was not very easy because in case we want to check this pattern in a function:

(drop
  (i32.add
    (local.get $x)
    (i32.const 3)
  )
)

Simply doing the below doesn't work in a lot of times

;; CHECK:     (drop
;; CHECK-NEXT:  (i32.add
;; CHECK-NEXT:    (local.get $x)
;; CHECK-NEXT:    (i32.const 3)
;; CHECK-NEXT:  )
;; CHECK-NEST: )

because the first (drop can be matched to another line that comes before the part we want to match, and all those CHECK-NEXT will fail. So I found, unlike LLVM in which we don't use autogeneration very often, using the autogeneration script to generate CHECK lines for a whole function was easier. But I think this is still better than .txt files in readability.

Comment thread test/lit/passes/dce-eh.wast Outdated
;; ---------------------------------------------------------------------------
(import "a" "b" (func $foo (result i32)))
(event $event$0 (attr 0) (param i32))
(func $callee-with-label

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why doesn't this function have check lines? If they were manually deleted, they will come back next time someone runs ./auto_update_tests.py, so it would be better just to keep them in.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is strange. update_lit_checks.py actually doesn't update this. Also (func $callee in the same file. Not sure why.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must be a bug in the regex used by the script. I wouldn't worry about it if those functions aren't critical to the test.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This indeed can't exist because $callee-with-label is inlined! 🙃

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohhh, that makes sense. Sorry for the confusion!

Co-authored-by: Thomas Lively <7121787+tlively@users.noreply.github.com>
@aheejin
aheejin merged commit eac90b5 into WebAssembly:main Jun 8, 2021
@aheejin
aheejin deleted the eh_lit_tests branch June 8, 2021 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants