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

[craig.topper]

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.

[efriedma]

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

[craig.topper]

Ping

[atanasyan]

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

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

SDValue DAGCombiner::visitSTORE(SDNode *N) {
   SDValue Ptr   = ST->getBasePtr();
 
   // If this is a store of a bit convert, store the input value if the
   // resultant store does not need a higher alignment than the original.
   if (Value.getOpcode() == ISD::BITCAST && !ST->isTruncatingStore() &&
       ST->isUnindexed()) {
     EVT SVT = Value.getOperand(0).getValueType();
     if (((!LegalOperations && !ST->isVolatile()) ||
-         TLI.isOperationLegalOrCustom(ISD::STORE, SVT)) &&
+         TLI.isOperationLegal(ISD::STORE, SVT)) &&
         TLI.isStoreBitCastBeneficial(Value.getValueType(), SVT)) {
       unsigned OrigAlign = ST->getAlignment();
       bool Fast = false;
       if (TLI.allowsMemoryAccess(*DAG.getContext(), DAG.getDataLayout(), SVT,
                                  ST->getAddressSpace(), OrigAlign, &Fast) &&
           Fast) {
         return DAG.getStore(Chain, SDLoc(N), Value.getOperand(0), Ptr,
                             ST->getPointerInfo(), OrigAlign,

test/CodeGen/Mips/cconv/arguments-hard-float-varargs.ll

 ; ALL-LABEL: float_args:
 ; We won't test the way the global address is calculated in this test. This is
 ; just to get the register number for the other checks.
 ; SYM32-DAG:         addiu [[R2:\$[0-9]+]], ${{[0-9]+}}, %lo(floats)
 ; SYM64-DAG:         daddiu [[R2:\$[0-9]+]], ${{[0-9]+}}, %lo(floats)
 
 ; The first four arguments are the same in O32/N32/N64.
 ; The non-variable portion should be unaffected.
-; O32-DAG:           sw $4, 4([[R2]])
+; O32-DAG:           mtc1 $4, $f0
+; O32-DAG:           swc1 $f0, 4([[R2]])
 ; NEW-DAG:           swc1 $f12, 4([[R2]])
 
 ; The varargs portion is dumped to stack
 ; O32-DAG:           sw $5, 12($sp)
 ; O32-DAG:           sw $6, 16($sp)
 ; O32-DAG:           sw $7, 20($sp)
 ; NEW-DAG:           sd $5, 8($sp)
 ; NEW-DAG:           sd $6, 16($sp)

test/CodeGen/Mips/cconv/arguments-hard-float.ll

 ; ALL-DAG:           swc1 $f12, 4([[R1]])
 ; O32-DAG:           swc1 $f14, 8([[R1]])
 ; NEW-DAG:           swc1 $f13, 8([[R1]])
 
 ; O32 has run out of argument registers and (in theory) starts using the stack
 ; I've yet to find a reference in the documentation about this but GCC uses up
 ; the remaining two argument slots in the GPR's first. We'll do the same for
 ; compatibility.
-; O32-DAG:           sw $6, 12([[R1]])
+; O32-DAG:           mtc1 $6, $f0
+; O32-DAG:           swc1 $f0, 12([[R1]])
 ; NEW-DAG:           swc1 $f14, 12([[R1]])
-; O32-DAG:           sw $7, 16([[R1]])
+; O32-DAG:           mtc1 $7, $f0
+; O32-DAG:           swc1 $f0, 16([[R1]])
 ; NEW-DAG:           swc1 $f15, 16([[R1]])
 
 ; O32 is definitely out of registers now and switches to the stack.
 ; O32-DAG:           lwc1 [[F1:\$f[0-9]+]], 16($sp)
 ; O32-DAG:           swc1 [[F1]], 20([[R1]])
 ; NEW-DAG:           swc1 $f16, 20([[R1]])
 ; O32-DAG:           lwc1 [[F1:\$f[0-9]+]], 20($sp)
 ; O32-DAG:           swc1 [[F1]], 24([[R1]])

 ; The first argument is the same in O32/N32/N64.
 ; ALL-DAG:           sb $4, 1([[R1]])
 
 ; The first argument isn't floating point so floating point registers are not
 ; used in O32, but N32/N64 will still use them.
 ; MD00305 and GCC disagree on this one. MD00305 says that floats are treated
 ; as 8-byte aligned and occupy two slots on O32. GCC is treating them as 4-byte
 ; aligned and occupying one slot. We'll use GCC's definition.
-; O32-DAG:           sw $5, 4([[R2]])
+; O32-DAG:           mtc1 $5, $f0
+; O32-DAG:           swc1 $f0, 4([[R2]])
 ; NEW-DAG:           swc1 $f13, 4([[R2]])