search

google / benchmark

A microbenchmark support library
Apache License 2.0
8.93k stars 1.62k forks source link

Enable vectors in `DoNotOptimize` #1738

Closed goldsteinn closed 8 months ago

goldsteinn commented 8 months ago

Just a QOL change to make benchmarks vector code easier.

LebedevRI commented 8 months ago

Tests?

goldsteinn commented 8 months ago

Tests?

Do you want aarch64 + x86_64, or is just x86_64 enough.

LebedevRI commented 8 months ago

Tests?

Do you want aarch64 + x86_64, or is just x86_64 enough.

I don't really know what kind of input variable type this targets so i can't tell what exactly should be tested. Can this be generically reached via clang vector types e.g.?

goldsteinn commented 8 months ago

Tests?

Do you want aarch64 + x86_64, or is just x86_64 enough.

I don't really know what kind of input variable type this targets so i can't tell what exactly should be tested. Can this be generically reached via clang vector types e.g.?

Done, tests only compiled for gcc/clang.

LebedevRI commented 8 months ago

Some generic questions:

  1. Has that asm constraint always been supported in every compiler version ever released?
  2. What happens when it's used on other compilers? (MSVC?)
  3. Does the order matter? Should it be r,v,m (i.e. mem being last)?
goldsteinn commented 8 months ago

Some generic questions:

  1. Has that asm constraint always been supported in every compiler version ever released?
  2. What happens when it's used on other compilers? (MSVC?)
  3. Does the order matter? Should it be r,v,m (i.e. mem being last)?

Yeah, this patch is a bad idea. Its not super portable AFAICT (across compilers or arch within).

LebedevRI commented 8 months ago

Aha, so clang 3.9 didn't know that: https://godbolt.org/z/eP9zbjb4P And even then, it's support is target dependent: https://godbolt.org/z/v58edcaYT This, indeed, makes me uneasy about this feature, unfortunately...