On Sun, Jan 6, 2013 at 5:22 PM, Jakub Jelinek <ja...@redhat.com> wrote:

>> --- df-problems.c     (revision 194945)
>> +++ df-problems.c     (working copy)
>> @@ -3916,6 +3916,10 @@ can_move_insns_across (rtx from, rtx to, rtx acros
>>       break;
>>        if (NONDEBUG_INSN_P (insn))
>>       {
>> +       /* Do not move unspec_volatile insns.  */
>> +       if (GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE)
>> +         break;
>> +
>
> Shouldn't UNSPEC_VOLATILE be handled similarly in the across_from ..
> across_to loop?  Both UNSPEC_VOLATILE and volatile asm are handled there
> just with
>         trapping_insns_in_across |= may_trap_p (PATTERN (insn));
> but your new change doesn't prevent moving just trapping insns across
> UNSPEC_VOLATILE, but any insns whatsoever.  So supposedly for UNSPEC_VOLATILE
> the first loop should just return false; (or fail = 1; ?).
> For asm volatile I guess the code is fine as is, it must always describe
> what exactly it modifies, so supposedly non-trapping insns can be moved
> across asm volatile.
>
>>         if (may_trap_or_fault_p (PATTERN (insn))
>>             && (trapping_insns_in_across || other_branch_live != NULL))
>>           break;
>
> You could do the check only for may_trap_or_fault_p, all UNSPEC_VOLATILE
> may trap.
>
> BTW, can't UNSPEC_VOLATILE be embedded deeply in the pattern?
> So volatile_insn_p (insn) && asm_noperands (PATTERN (insn)) == -1?
> But perhaps you want to treat that way only UNSPEC_VOLATILE directly in the
> pattern and all other UNSPEC_VOLATILE insns must describe in detail what
> exactly they are changing?  This really needs to be better documented.

TBH, I'm not that familiar with the RTL infrastructure enough to
answer these questions. While I can spend some time on this problem,
and probably waste quite some reviewer's time, the problem is not that
trivial as I hoped to be, so I would kindly ask someone with better
understanding of this part of the compiler for the proper solution.

Uros.

Reply via email to