Ian Lance Taylor wrote:
> Georg-Johann Lay:
>>
>> What about that predicate?
>>
>> (define_predicate "low_io_mem"
>>    (and (match_code "mem")
>>         (match_test "low_io_address_operand (XEXP (op, 0))")))
>>
>> Guess it operates the same because the drag-volatile-over-volatile
>> happens not in recog(_for_combine) but already when combine
>> synthesizes new patterns.

Jepp, I tried that. Just as I expected, it is no cure.

>> As combine juggles with data flow, it must
>> not exchange volatiles. That's all. The backend will know if it's ok
>> to place a specific operand into a specific insn or not.
>>
>> And skipping optimizations like these altogether because they are
>> uncomfortable to handle is the wrong direction, IMHO.
> 
> OK, what is your proposed patch to combine?  Maybe it is easy to make it
> do what you want.  I haven't looked.
> 
> Ian

No, I have no patch. But I had a closer look at combine.c:

In try_combine, just after the "Trying..." dump output, we have:

i0 = 0
i1 = 0
i2 =

(set (reg/v:QI 43 [ status ])
     (mem/v:QI (const_int 43 [0x2b])))

i3 =

(set (pc)
     (if_then_else (eq (zero_extract:QI (reg/v:QI 43 [ status ])
                                        (const_int 1)
                                        (const_int 4))
                       (const_int 0))
                   (label_ref:HI 16)
                   (pc)))

where to potential insertion is i2 into i3.

These insns are fed into can_combine_p with

src  = (mem/v:QI (const_int 43))
dest = (reg/v:QI 43)

and then there is this part of an if-clause:

      /* Make sure that the value that is to be substituted for the register
         does not use any registers whose values alter in between.  However,
         If the insns are adjacent, a use can't cross a set even though we
         think it might (this can happen for a sequence of insns each setting
         the same destination; last_set of that register might point to
         a NOTE).  If INSN has a REG_EQUIV note, the register is always
         equivalent to the memory so the substitution is valid even if there
         are intervening stores.  Also, don't move a volatile asm or
         UNSPEC_VOLATILE across any other insns.  */
      || (! all_adjacent
          && (((!MEM_P (src)
                || ! find_reg_note (insn, REG_EQUIV, src))
               && use_crosses_set_p (src, DF_INSN_LUID (insn)))
              || (GET_CODE (src) == ASM_OPERANDS && MEM_VOLATILE_P (src))
              || GET_CODE (src) == UNSPEC_VOLATILE))


In addition to these tests, the following must be disallowed:

If src contains volatile memory, then disallow moving it across:

* volatile memory
* unspec_volatile
* asm volatile

As far as I can see, use_crosses_set_p (src,...) returns 0 (false) which is
incorrect.

So either use_crosses_set_p is incorrect or it relies on incorrect data from
data flow analysis or from wherever.

Johann

Reply via email to