Skip to content

Commit

Permalink
Compilation error with gcc-10.
Browse files Browse the repository at this point in the history
  • Loading branch information
osschar committed Jan 11, 2021
1 parent ddeea75 commit 27f790c
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion Event.cc
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ void Event::write_out(DataFile &data_file)
//sizes are the same as in layerHits_
for (int il = 0; il<nl; ++il) {
int nh = layerHitMasks_[il].size();
assert(nh == layerHits_[il].size());
assert(nh == (int) layerHits_[il].size());
fwrite(&layerHitMasks_[il][0], sizeof(uint64_t), nh, fp);
evsize += nh*sizeof(uint64_t);
}
Expand Down

3 comments on commit 27f790c

@srlantz
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same error is present in the stable-2020 branch. Should this commit be pushed to that branch, as well?

As long as I'm commenting, let me add that gcc-9 finds this error too... and it issues plenty of warnings besides. Maybe we should put together a somewhat larger commit that addresses those issues, along with this one? I believe our Makefile.config is set up to flag the same sorts of issues that would normally be flagged by CMS.

@dan131riley
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe our Makefile.config is set up to flag the same sorts of issues that would normally be flagged by CMS.

Yes, the gcc flags in Makefile.config are mostly copied from the CMSSW flags, though they are likely somewhat out of date.

@osschar
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did direct edit on stable-2020.

I think all the warnings come from magick in Ice/RevisitedRadix ... that does radix sort of floating point numbers and does some pretty weird stuff converting between integer and floating-point representation.

Now, Slava has already changed hit sorting to use integer radix sort ... and seeing that code, we could do the same for sorting of seeds (i.e., splitting an int into major / minor sort bitfield). Oh, and BTW, we really should also sort seeds like this before cleaning (and maybe final tracks before candidate removal).

Alternatively, we could tweak warnings for mkFit/Ice file.

Oh boy, apparently he made a review of the code in 2018, and cache usage improvements for sorting more than 1M entries :)
https://github.com/Pierre-Terdiman/RadixRedux
https://github.com/Pierre-Terdiman/RadixRedux/blob/master/RadixRedux.pdf

Please sign in to comment.