Page MenuHomePhabricator

Pass -fprofile-sample-use to lto backends.
ClosedPublic

Authored by danielcdh on Jan 11 2017, 4:45 PM.

Details

Event Timeline

danielcdh updated this revision to Diff 84056.Jan 11 2017, 4:45 PM
danielcdh retitled this revision from to Pass -fprofile-sample-use to lto backends..
danielcdh updated this object.
danielcdh added reviewers: tejohnson, mehdi_amini.
danielcdh added a subscriber: cfe-commits.
tejohnson accepted this revision.Jan 11 2017, 7:40 PM
tejohnson edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jan 11 2017, 7:40 PM
danielcdh closed this revision.Jan 12 2017, 8:40 AM
danielcdh reopened this revision.Jan 12 2017, 2:23 PM

The breaks some buildbots thus I reverted the patch:

http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/1889
http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-incremental_check/32242/

Unfortunately I could not reproduce the error locally.

Any quick insights why this test change would break?

Thanks,
Dehao

This revision is now accepted and ready to land.Jan 12 2017, 2:23 PM

The breaks some buildbots thus I reverted the patch:

http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/1889
http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-incremental_check/32242/

Unfortunately I could not reproduce the error locally.

Any quick insights why this test change would break?

Looks like there was no output (at least on the buildbot that I looked at). Most likely because this is using debug output and perhaps those are build NDEBUG? I wonder if there is a better way to test this. Otherwise I think "; REQUIRES: asserts" might do the trick?

Thanks,
Dehao

Otherwise I think "; REQUIRES: asserts" might do the trick?

Looks like it - I looked at another test that used -debug output and it requires asserts.

Thanks for the prompt response.

But looks like several other tests also has "-mllvm -debug-pass=Structure" in their tests:

tools/clang/test/CodeGen/pgo-instrumentation.c
test/CodeGen/Generic/llc-start-stop.ll

I just verified that if I cmake with -DCMAKE_BUILD_TYPE=Release, it still passes locally.

Thanks,
Dehao

Thanks for the prompt response.

But looks like several other tests also has "-mllvm -debug-pass=Structure" in their tests:

tools/clang/test/CodeGen/pgo-instrumentation.c
test/CodeGen/Generic/llc-start-stop.ll

I just verified that if I cmake with -DCMAKE_BUILD_TYPE=Release, it still passes locally.

Hmm, maybe that flavor of -debug output is enabled with NDEBUG then. I'm not really sure unfortunately.

I don't even see an error message for the second one, do you? Just an error code.

Thanks,
Dehao

danielcdh updated this revision to Diff 84193.Jan 12 2017, 5:01 PM
danielcdh edited edge metadata.

Updates the unittests to clang_cc1 to see if it fixes the buildbot failure.

danielcdh closed this revision.Jan 12 2017, 5:02 PM

Looks like this is still breaking these buildbots:

http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/3216/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Athinlto_backend.ll

I reverted the test change for now, and am thinking of how to reproduce/fix the problem...

Looks like this is still breaking these buildbots:

http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/3216/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Athinlto_backend.ll

I reverted the test change for now, and am thinking of how to reproduce/fix the problem...

Aha - the test is missing "-target x86_64-unknown-linux-gnu". Probably because it was cloned from the one just above or below it, which are testing for other failures hit early on and didn't need it (they should probably have it as well to avoid confusion like this in the future though).