David Miller <da...@davemloft.net> writes:
> From: David Miller <da...@davemloft.net>
> Date: Tue, 23 Oct 2012 21:44:05 -0400 (EDT)
>
>> The first issue sparc runs into is that it does not define it's
>> extra constraints properly.  In particular 'T' and 'W' must use
>> define_memory_constraint.
>> 
>> Otherwise the EXTRA_MEMORY_CONSTRAINT logic in process_alt_operands()
>> does not trigger and we therefore cannot even load a constant into
>> floating point registers properly.
>
> Ok, the next problem we hit will be a little bit more difficult
> to solve.
>
> Sparc accepts addresses of the form:
>
> (plus:DI (lo_sum:DI (reg/f:DI 282)
>         (symbol_ref:DI ("__mf_opts") <var_decl 0xf78d74a0 __mf_opts>))
>     (const_int 40 [0x28]))
>
> These make use of Sparc's offsetable %lo() relocations.

Hmm, this looks a bit risky.  In terms of RTL semantics, this
(plus:DI ...) is a full 64-bit addition of the result of the
(lo_sum:DI ...), whatever that (lo_sum:DI ...) result happens to be.
I assume the offset is really folded into the %lo() constant itself,
in which case the usual form would be:

(lo_sum:DI (reg/f:DI 282)
           (const:DI (plus:DI (symbol_ref:DI ...) (const_int 40))))

That's the form created by e.g. adjust_address_1 in cases where it can
prove that adding the offset won't trigger a carry into the high part:

      /* If MEMREF is a LO_SUM and the offset is within the alignment of the
         object, we can merge it into the LO_SUM.  */
      if (GET_MODE (memref) != BLKmode && GET_CODE (addr) == LO_SUM
          && offset >= 0
          && (unsigned HOST_WIDE_INT) offset
              < GET_MODE_ALIGNMENT (GET_MODE (memref)) / BITS_PER_UNIT)
        addr = gen_rtx_LO_SUM (address_mode, XEXP (addr, 0),
                               plus_constant (address_mode,
                                              XEXP (addr, 1), offset));

(which, now I'm quoting it, probably isn't correct for extreme
alignments, but still).

In other words, the LO_SUM constant can have an extra offset over and
above the HIGH constant in cases where we "know" that adding the extra
offset doesn't trigger a carry.

The danger with matching PLUSes of LO_SUMs is that it would be valid
for combine to turn:

    (set (reg:DI X1) (high:DI (symbol_ref:DI "foo")))
    (set (reg:DI X2) (plus:DI (reg:DI X1) (...a variable index...)))
    (set (reg:DI X3) (lo_sum:DI (reg:DI X2) (symbol_ref:DI "foo")))
    ...(mem:DI (plus:DI (reg:DI X3) (const_int 128))) ...

into:

    (set (reg:DI X1) (high:DI (symbol_ref:DI "foo")))
    (set (reg:DI X2) (plus:DI (reg:DI X1) (...a variable index...)))
    ...(mem:DI (plus:DI (lo_sum:DI (reg:DI X2) (symbol_ref:DI "foo"))
                        (const_int 128)))...

even in cases where foo+128 triggers a carry into the high-part relocation.
I assume that would produce wrong code, since the carry won't be
represented in the assignment to X1.  On the other hand, combine
couldn't transform this to:

    (set (reg:DI X1) (high:DI (symbol_ref:DI "foo")))
    (set (reg:DI X2) (plus:DI (reg:DI X1) (...a variable index...)))
    ...(mem:DI (lo_sum:DI (reg:DI X2) (const:DI (plus:DI (symbol_ref:DI "foo"))
                                                (const_int 128)))) ...

without proving the lack of a carry.

FWIW, MIPS just accepts the (lo_sum ... (const ...)) form and I've
not noticed any problems.  In particular, doubleword loads do use:

        lui     $4,%hi(foo)
        lw      $6,%lo(foo)($4)
        lw      $7,%lo(foo+4)($4)

as hoped.  There might well be other rtl optimisations that need
to know about this though.

I suppose this comes back to what Vlad was saying about the choice
between (a) trying to make LRA work with current backends as far
as possible and (b) not adding workarounds or complications to
LRA in cases where other code really ought to change instead.
The reviews I did were definitely pushing in the (b) direction.
(Although in fairness, I think the original LRA code could have
miscompiled this kind of thing instead of aborting, because the
disp_loc would have been set to one constant or the other.)

Richard

Reply via email to