Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Declare liblogintpack functions as inline #14014

Merged
merged 1 commit into from Apr 15, 2016

Conversation

makortel
Copy link
Contributor

As the functions are defined in a header, they should be declared inline. Should affect only testDataFormatsPatCandidates, which

  • #includes the header directly testlogintpack.cc
  • links a library (DataFormats/PatCandidates) that has a source #including the header

Without declaring the functions inline, if one adds <use name="vdt"/> to DataFormats/PatCandidates/BuildFile.xml, the std::exp in unpack8logClosed gives 1 ulp difference for unpack8logClosed(-1, -15, 0) wrt. not "using" vdt. Note that the effect is visible only when calling the unpack8logClosed function. If I copy-paste the contents to the test program, I get consistently the "not-using-vdt" values. With inline the return value of unpack8logClosed is does not depend on linking against vdt.

With this PR, the (older version of) #13959 (with DataFormats/ParticleFlowReco/BuildFile.xml using vdt instead of vdt_headers) would not cause testDataFormatsPatCandidates to fail.

Tested in 8_1_0_pre2, no changes expected.

@VinInn

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @makortel (Matti Kortelainen) for CMSSW_8_1_X.

It involves the following packages:

DataFormats/PatCandidates

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@gpetruc this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@VinInn
Copy link
Contributor

VinInn commented Apr 11, 2016

@cmsbuild , please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/12284/console

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@cvuosalo
Copy link
Contributor

+1

For #14014 a9b9bc2

Inlining liblogintpack functions to prevent inconsistent behavior. There should be no change in monitored quantities.

The code changes are satisfactory, and Jenkins tests against baseline CMSSW_8_1_X_2016-04-11-1100 show no significant differences, as expected.

@davidlange6 davidlange6 merged commit d1095bd into cms-sw:CMSSW_8_1_X Apr 15, 2016
@makortel makortel deleted the inlineLiblongintpack branch April 19, 2017 09:46
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.

None yet

5 participants