This is an archive of the discontinued LLVM Phabricator instance.

test-suite: add cpu features detection in configuration
ClosedPublic

Authored by itaraban on Sep 22 2017, 9:29 AM.

Details

Summary

This patch adds opportunity to detect cpu features via configuration, which can be used later in Makefiles. I'm preparing some tests, which use specific cpu features, so I don't want them to fail on machines without such features. Unfortunately, most of buildbots use configure+make approach, so I propose this patch for configure and makefiles structure.
This additional m4 script is based on "ax_gcc_x86_cpu_support" and "ax_check_x86_features", scripts from autoconf base.

Patch with similar feature for cmake - D38484

Diff Detail

Repository
rL LLVM

Event Timeline

itaraban created this revision.Sep 22 2017, 9:29 AM
itaraban edited the summary of this revision. (Show Details)
eladcohen added a subscriber: llvm-commits.
MatzeB edited edge metadata.Sep 25 2017, 8:18 AM

Using autoconf+Makefiles to run the test-suite is deprecated; Most people use cmake/ninja/lit to run it nowadays (or lnt runtest test-suite which uses cmake under the hood). Do you plan to support the feature detection on cmake as well?

Yes, I know that fact, but some buildbots use this approach(lnt runtest nt). They all use this approach until https://reviews.llvm.org/D35402. So some of them(17 buildbots) still use nt instead of test-suite. That's why I forced to make support for deprecated method.
Sure, I also plan to add cpu type detection in cmake in next patch.

This looks mostly fine.

  • What about cross compilation scenarios? I think this should have a way to manually set the flags in that case.
  • We need equivalent tests on the cmake side.
  • This should only be committed when we have some tests that actually use it.
autoconf/m4/get_cpu_target.m4
54 ↗(On Diff #116359)

This should have X86 in the name, and probably shouldn't even do anything if the target triple isn't x86.

MatzeB added inline comments.Sep 26 2017, 2:44 PM
Makefile.config.in
234–250 ↗(On Diff #116359)

Please insert an X86 into the names.

itaraban updated this revision to Diff 116777.Sep 27 2017, 2:11 AM

What about cross compilation scenarios? I think this should have a way to manually set the flags in that case.

You can enable or disable specific instructions manually via make command arguments.

make "HAVE_X86_AVX512F_INSTRUCTIONS=1" ...
itaraban edited the summary of this revision. (Show Details)Oct 2 2017, 5:21 PM
MatzeB accepted this revision.Oct 2 2017, 5:34 PM

Tentatively approving this, but please hold on with committing until we have a patch approved that actually uses this.

This revision is now accepted and ready to land.Oct 2 2017, 5:34 PM

Patch which uses this feature: D41249

LGTM still stands.

This revision was automatically updated to reflect the committed changes.