This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombiner][Mips] Don't combine bitcast+store after LegalOperations when the store is volatile, if the resulting store isn't Legal
ClosedPublic

Authored by craig.topper on Aug 10 2018, 12:39 PM.

Details

Summary

Previously we allowed the store to be Custom. But without knowing for sure that the Custom handling won't split the store, we shouldn't convert a volatile store. We also probably shouldn't be creating a store the requires custom handling after LegalizeOps. This could lead to an infinite loop if the custom handling was to insert a bitcast. Though I guess isStoreBitCastBeneficial could be used to block such a loop.

The test changes here are due to the volatile part of this. The stores in the test are all volatile and i32 stores are marked custom, So we are no longer converting them

This is related to D50491 where I was trying to allow some bitcasting of volatile loads.

Diff Detail

Repository
rL LLVM

Event Timeline

craig.topper created this revision.Aug 10 2018, 12:39 PM

Code change LGTM, but someone familiar with the MIPS backend should review the test changes.

atanasyan accepted this revision.Aug 23 2018, 1:13 AM

LGTM
MIPS code becomes a bit longer, but I think it can be fixed in the future.

This revision is now accepted and ready to land.Aug 23 2018, 1:13 AM
This revision was automatically updated to reflect the committed changes.