This is an archive of the discontinued LLVM Phabricator instance.

[CodeGenPrepare] Don't split the store if it is volatile
ClosedPublic

Authored by steven.zhang on Apr 25 2019, 7:22 PM.

Diff Detail

Event Timeline

steven.zhang created this revision.Apr 25 2019, 7:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 25 2019, 7:22 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
jsji retitled this revision from [CodeGen] When split the store, get the volatile flag from the old store to [CodeGenPrepare] When split the store, get the volatile flag from the old store.Apr 26 2019, 8:07 AM
jsji added a reviewer: spatel.

Hello. I feel like this splitting just shouldn't be done for volatile stores.

steven.zhang retitled this revision from [CodeGenPrepare] When split the store, get the volatile flag from the old store to [CodeGenPrepare] Don't split the store if it is volatile.
steven.zhang edited the summary of this revision. (Show Details)

Hello. I feel like this splitting just shouldn't be done for volatile stores.

Good point. Address the comments.

Looks like there is similar code in DAGCombiner that may need a volatile check too, if you search for isMultiStoresCheaperThanBitsMerge.

llvm/test/CodeGen/PowerPC/splitstore-check-volatile.ll
4

You could just make this an opt test, running the codegenprepare pass.

Use opt instead of llc to do the check.

dmgreen accepted this revision.May 6 2019, 3:20 PM

This LGTM.

Like I said, you may also want to look into doing the same thing for the code in DAGCombiner.

This revision is now accepted and ready to land.May 6 2019, 3:20 PM
This revision was automatically updated to reflect the committed changes.