Page MenuHomePhabricator

Allow unordered loads to be considered invariant in CodeGen
ClosedPublic

Authored by reames on Mar 14 2019, 11:13 AM.

Details

Summary

The actual code change is fairly straight forward, but exercising it isn't. First, it turned out we weren't adding the appropriate flags in SelectionDAG. Second, it turned out that we've got some optimization gaps, so obvious test cases don't work.

My first attempt (in atomic-unordered.ll) points out a deficiency in our peephole-opt folding logic which I plan to fix separately. Instead, I'm exercising this through MachineLICM.

Diff Detail

Repository
rL LLVM

Event Timeline

reames created this revision.Mar 14 2019, 11:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 14 2019, 11:13 AM
jfb added a comment.Mar 14 2019, 11:23 AM

This part isn't something I've very familiar with, better if someone else reviews.

lib/CodeGen/MachineInstr.cpp
1319 ↗(On Diff #190682)

Do you ever have volatile unordered?

jlebar accepted this revision.Mar 18 2019, 2:57 PM

LGTM modulo jfb's outstanding comment.

lib/CodeGen/MachineInstr.cpp
1318 ↗(On Diff #190682)

"need to be updated"?

This revision is now accepted and ready to land.Mar 18 2019, 2:57 PM
reames marked an inline comment as done.Mar 19 2019, 11:17 AM
reames added inline comments.
lib/CodeGen/MachineInstr.cpp
1319 ↗(On Diff #190682)

isUnordered returns false for volatiles.

reames marked an inline comment as done.Mar 19 2019, 11:19 AM
reames added inline comments.
lib/CodeGen/MachineInstr.cpp
1318 ↗(On Diff #190682)

I left the wording here because this is a possible change with possible downsides design wise. I don't what to give the impression it's a straight forward change.

This revision was automatically updated to reflect the committed changes.