Page MenuHomePhabricator

Suppress FP_CONTRACT due to planned command line changes (changes in the Makefile).
ClosedPublic

Authored by zahiraam on Jul 23 2021, 12:06 PM.

Details

Summary

In https://reviews.llvm.org/D102861 changes to the LNT suite were made to accommodate the clang patch in https://reviews.llvm.org/D74436 . These changes were made in the CMakeLists.txt. Unfortunately the buildbot seems to be running make. This patch is to add those same (FLAGS) changes in D102861 to the Makefiles.

Diff Detail

Event Timeline

zahiraam created this revision.Jul 23 2021, 12:06 PM
zahiraam requested review of this revision.Jul 23 2021, 12:06 PM
mibintc accepted this revision.Jul 23 2021, 12:16 PM

LGTM - please see changes requested inline. thanks a lot

SingleSource/Benchmarks/Misc-C++/Large/Makefile
3

I think these should all say "+=" instead of merely "=" because there might be an initialization of flags from somewhere else e.g. the Make command line invocation. If you use += then it will add onto the flags, whereas straight assignment I believe would lose the other setting.

If there's no particular reason why some files use CPPFLAGS and others divide into CXXFLAGS and CFLAGS why not use the same setting in each file (i.e. choose one way or the other)

I'm going to go ahead and +1 this assuming you will make these changes

LGTM thanks so much for doing this!

This revision is now accepted and ready to land.Jul 23 2021, 12:16 PM
zahiraam added inline comments.Jul 23 2021, 12:26 PM
SingleSource/Benchmarks/Misc-C++/Large/Makefile
3

I will change all these to +=.
The reason I used 2 different ways of doing is that in SingleSource/Make.singlesrc, there is a rule at line#80 that is using the CFLAGS but not the CPPFLAGS so wanted to make sur that those flags will be used regardless of the CPPFLAGS.

zahiraam added inline comments.Jul 23 2021, 12:32 PM
SingleSource/Benchmarks/Misc-C++/Large/Makefile
3

But then I see that MultiSource/Makefile.multisrc also has a rule that doesn't use CPPFLAGS with CFLAGS (line 95).

mibintc added inline comments.Jul 23 2021, 12:38 PM
SingleSource/Benchmarks/Misc-C++/Large/Makefile
3

Probably doesn't matter, just being picky. I'm good with whatever you choose. thanks again

zahiraam updated this revision to Diff 361366.Jul 23 2021, 3:22 PM
mibintc accepted this revision.Jul 24 2021, 5:15 AM
zahiraam set the repository for this revision to rT test-suite.Jul 26 2021, 6:34 AM
zahiraam updated this revision to Diff 361644.Jul 26 2021, 7:15 AM
zahiraam updated this revision to Diff 361738.Jul 26 2021, 11:41 AM
zahiraam set the repository for this revision to rT test-suite.
zahiraam removed rT test-suite as the repository for this revision.Jul 26 2021, 2:11 PM
mibintc accepted this revision.Jul 26 2021, 4:18 PM
mibintc closed this revision.Jul 29 2021, 5:32 AM

commit 79f2b03c5111392d647fa542e120f70c8f39a991
Author: Zahira Ammarguellat <zahira.ammarguellat@intel.com>
Date: Mon Jul 26 11:36:23 2021 -0700

Closed by this commit (fixed) the buildbots are working well for Intel thanks Zahira