Skip to content

Add tests to verify assembler output -- Fix DoNotOptimize.#530

Merged
EricWF merged 5 commits intogoogle:masterfrom
efcs:add-assembly-tests
Mar 23, 2018
Merged

Add tests to verify assembler output -- Fix DoNotOptimize.#530
EricWF merged 5 commits intogoogle:masterfrom
efcs:add-assembly-tests

Conversation

@EricWF
Copy link
Copy Markdown
Contributor

@EricWF EricWF commented Feb 14, 2018

For things like DoNotOptimize, ClobberMemory, and even KeepRunning(),
it is important exactly what assembly they generate. However, we currently
have no way to test this. Instead it must be manually validated every
time a change occurs -- including a change in compiler version.

This patch attempts to introduce a way to test the assembled output automatically.
It's mirrors how LLVM verifies compiler output, and it uses LLVM FileCheck to run
the tests in a similar way.

The tests function by generating the assembly for a test in CMake, and then
using FileCheck to verify the // CHECK lines in the source file are found
in the generated assembly.

Currently, the tests only run on 64-bit x86 systems under GCC and Clang,
and when FileCheck is found on the system.

Additionally, this patch tries to improve the code gen from DoNotOptimize. This should probably be a separate change, but I needed something to test.

This PR is largely just to run the new patch against the bots to ensure nothing has broken.

@EricWF EricWF force-pushed the add-assembly-tests branch 2 times, most recently from 0456059 to b82654b Compare February 14, 2018 05:03
@AppVeyorBot
Copy link
Copy Markdown

Build benchmark 1038 completed (commit ea9de3658c by @EricWF)

@EricWF EricWF force-pushed the add-assembly-tests branch from 822f76d to b2d8318 Compare February 14, 2018 05:56
@google google deleted a comment from AppVeyorBot Feb 14, 2018
@google google deleted a comment from AppVeyorBot Feb 14, 2018
@google google deleted a comment from coveralls Feb 14, 2018
@google google deleted a comment from AppVeyorBot Feb 14, 2018
@google google deleted a comment from AppVeyorBot Feb 14, 2018
@EricWF EricWF force-pushed the add-assembly-tests branch 3 times, most recently from 1fd7610 to d207257 Compare February 14, 2018 06:10
@EricWF EricWF added the RFC label Feb 14, 2018
@AppVeyorBot
Copy link
Copy Markdown

Build benchmark 1047 completed (commit 4b063052b6 by @EricWF)

@AppVeyorBot
Copy link
Copy Markdown

Build benchmark 1055 completed (commit afed53d504 by @EricWF)

@AppVeyorBot
Copy link
Copy Markdown

Build benchmark 1056 failed (commit 907711ef1d by @EricWF)

@AppVeyorBot
Copy link
Copy Markdown

Build benchmark 1057 completed (commit 428d4ca88c by @EricWF)

@EricWF EricWF force-pushed the add-assembly-tests branch from 3d87069 to 942cfd1 Compare February 14, 2018 09:57
@AppVeyorBot
Copy link
Copy Markdown

Build benchmark 1061 completed (commit 630e0e8506 by @EricWF)

@AppVeyorBot
Copy link
Copy Markdown

Build benchmark 1063 completed (commit af7ce5fdec by @EricWF)

@AppVeyorBot
Copy link
Copy Markdown

Build benchmark 1065 completed (commit 7738ee1ce5 by @EricWF)

@EricWF EricWF force-pushed the add-assembly-tests branch from 328869a to cc1136d Compare February 15, 2018 00:47
@AppVeyorBot
Copy link
Copy Markdown

Build benchmark 1069 completed (commit fb2552a0c2 by @EricWF)

@dmah42
Copy link
Copy Markdown
Member

dmah42 commented Feb 15, 2018

Can you add a markdown documentation file about AssemblyTests or whatever name you choose, essentially with the contents of your commit message. Maybe more detail.

And then cross-link to it from the README?

@pleroy
Copy link
Copy Markdown
Contributor

pleroy commented Feb 15, 2018

TIL about LLVM FileCheck. Pretty cool!

@EricWF
Copy link
Copy Markdown
Contributor Author

EricWF commented Feb 16, 2018

@dominichamon Yep. Can do.

@pleroy:

TIL about LLVM FileCheck. Pretty cool!

Indeed. It's an amazing tool to have when you need to test string output.

@EricWF
Copy link
Copy Markdown
Contributor Author

EricWF commented Feb 19, 2018

@dominichamon I added documentation as you requested. Please take a look at your leisure.

@AppVeyorBot
Copy link
Copy Markdown

Build benchmark 1070 completed (commit 52fd2436f5 by @EricWF)

@@ -0,0 +1,147 @@
# Assembly Tests

The Benchmark library provides a number of functions who's primary
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.

'whose'


```

#### LLVM Filecheck
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.

this should be ### i think, as you're only at ## above.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If it's no matter, I actually prefer having the smaller section headers for these bits. IMHO it's looks nicer and flows better.

@googlebot
Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

For things like `DoNotOptimize`, `ClobberMemory`, and even `KeepRunning()`,
it is important exactly what assembly they generate. However, we currently
have no way to test this. Instead it must be manually validated every
time a change occurs -- including a change in compiler version.

This patch attempts to introduce a way to test the assembled output automatically.
It's mirrors how LLVM verifies compiler output, and it uses LLVM FileCheck to run
the tests in a similar way.

The tests function by generating the assembly for a test in CMake, and then
using FileCheck to verify the // CHECK lines in the source file are found
in the generated assembly.

Currently, the tests only run on 64-bit x86 systems under GCC and Clang,
and when FileCheck is found on the system.

Additionally, this patch tries to improve the code gen from DoNotOptimize.
This should probably be a separate change, but I needed something to test.
@EricWF EricWF force-pushed the add-assembly-tests branch from dd39f2b to ca7f89c Compare March 22, 2018 23:23
@googlebot
Copy link
Copy Markdown

CLAs look good, thanks!

@EricWF
Copy link
Copy Markdown
Contributor Author

EricWF commented Mar 22, 2018

@dominichamon Any final words?

@AppVeyorBot
Copy link
Copy Markdown

Build benchmark 1130 completed (commit 408113083e by @EricWF)

@AppVeyorBot
Copy link
Copy Markdown

Build benchmark 1132 completed (commit 57aa030a13 by @EricWF)

@dmah42
Copy link
Copy Markdown
Member

dmah42 commented Mar 23, 2018

I'd prefer if the bazel TODO linked to a github issue so we can track it, but otherwise, Ship It.

@EricWF EricWF force-pushed the add-assembly-tests branch 2 times, most recently from 269be46 to 8e43961 Compare March 23, 2018 20:23
@EricWF EricWF force-pushed the add-assembly-tests branch from 8e43961 to 0f2f95f Compare March 23, 2018 21:01
@AppVeyorBot
Copy link
Copy Markdown

Build benchmark 1136 completed (commit ab6fe60e62 by @EricWF)

@EricWF EricWF merged commit 7b03df7 into google:master Mar 23, 2018
@EricWF EricWF deleted the add-assembly-tests branch March 23, 2018 22:11
JBakamovic pushed a commit to JBakamovic/benchmark that referenced this pull request Dec 6, 2018
* Add tests to verify assembler output -- Fix DoNotOptimize.

For things like `DoNotOptimize`, `ClobberMemory`, and even `KeepRunning()`,
it is important exactly what assembly they generate. However, we currently
have no way to test this. Instead it must be manually validated every
time a change occurs -- including a change in compiler version.

This patch attempts to introduce a way to test the assembled output automatically.
It's mirrors how LLVM verifies compiler output, and it uses LLVM FileCheck to run
the tests in a similar way.

The tests function by generating the assembly for a test in CMake, and then
using FileCheck to verify the // CHECK lines in the source file are found
in the generated assembly.

Currently, the tests only run on 64-bit x86 systems under GCC and Clang,
and when FileCheck is found on the system.

Additionally, this patch tries to improve the code gen from DoNotOptimize.
This should probably be a separate change, but I needed something to test.

* Disable assembly tests on Bazel for now

* Link FIXME to github issue

* Fix Tests on OS X

* fix strip_asm.py to work on both Linux and OS X like targets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants