This is an archive of the discontinued LLVM Phabricator instance.

[Target] Make a copy of TargetOptions feature list before sorting during CodeGen
ClosedPublic

Authored by craig.topper on Nov 19 2017, 1:05 PM.

Details

Summary

Currently CodeGen is calling std::sort on the features vector in TargetOptions for every function, but I don't think CodeGen should be modifying TargetOptions.

This patch makes a copy before sorting.

Diff Detail

Event Timeline

craig.topper created this revision.Nov 19 2017, 1:05 PM
echristo accepted this revision.Nov 20 2017, 3:21 PM

This seems strictly more difficult to keep under control? Though I guess the assert helps.

Feel free to go ahead, but...

This revision is now accepted and ready to land.Nov 20 2017, 3:21 PM

Yeah don't like it either, but was surprised to find that CodeGen even had a non-const reference to TargetOptions. Would it be better to make a copy and sort the copy during CodeGen?

No strong opinion. I think I like that one more though.

craig.topper retitled this revision from [Target] Keep the TargetOptions feature list sorted instead of sorting during CodeGen to [Target] Make a copy of TargetOptions feature list before sorting during CodeGen.
craig.topper edited the summary of this revision. (Show Details)

Make a copy before sorting and combine some repeated code that this exposed.

craig.topper planned changes to this revision.Nov 21 2017, 4:21 PM
craig.topper requested review of this revision.
This revision is now accepted and ready to land.Nov 21 2017, 4:21 PM
craig.topper requested review of this revision.Nov 21 2017, 4:57 PM
craig.topper edited edge metadata.
echristo accepted this revision.Nov 27 2017, 5:35 PM
This revision is now accepted and ready to land.Nov 27 2017, 5:35 PM
This revision was automatically updated to reflect the committed changes.