This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [lra] Cleanup the use of offmemok and don't count spilling cost for it




On 04/07/16 14:12, Bernd Schmidt wrote:
On 06/30/2016 07:24 PM, Jiong Wang wrote:
From my understanding, "offmemok" is used to represent a memory operand
who's address we want to reload, and searching of it's reference location
seems confirmed my understanding as it's always used together with MEM_P
check.

So this patch does the following modifications:

* Only set offmemok to true if MEM_P is also true, as otherwise offmemok
    is not used.
>   * Remove redundant MEM_P check which was used together with offmemok.

I really dislike this part. The various _ok variables say what is acceptable - the type of the operand doesn't really factor into that. I think the code becomes more confusing when merging the two.

* Avoid the addition of spilling cost if offmemok be true as an address
    calculation reload is not spilling.

This part seems to be plausible. I am however unclear how this would fix the ICE (if it does - Andreas?) since it only seems to modify cost computations. What exactly is preventing the correct sequence of events (reloading the address) from triggering without this patch?


Hi Bernd,

The ICE reported was a "Max. number of generated reload insns per insn
is achieved",

The input rtx pattern which triggers this issue is:

(insn 41 38 120 4 (set (mem:V4SI
        (plus:DI (plus:DI (reg/v/f:DI 116) (reg:DI 69 ))
                 (const_int -16))
     (subreg:V4SI (reg:V16QI 134) 0)) {movv4si}

And the corresponding s390 patten is "mov<mode>" for V_128.

(define_insn "mov<mode>"
  [(set (match_operand:V_128 0 "" "=v,v,R,  v,  v,  v,  v,  v,v,d")
          (match_operand:V_128 1 "" "v,R,v,j00,jm1,jyy,jxx,jKK,d,v"))]

As the offset "-16" does not qualify s390_short_displacement, we need a
reload.

Ideally we want alternative 2, for which gcc simply reload the mem
address into a address register.

r157:DI=r116:DI+r69:DI-0x10
[r157:DI]=r134:V16QI#0

While after r237277, gcc is treating the reload of insn 41 as a spill
and thus increased the costs for it, then alternative 8 beat alternative
2, thus the following reload sequences are generated.

r157:V4SI=r134:V16QI#0
[r116:DI+r69:DI-0x10]=r157:V4SI

GCC move the vector register into general register, then a second
instruction to store the general register into memory so it can match
alternative 8, which is "v", "d".

However the second instructions still constains the illegal mem address,
thus a further reload triggered, and gcc triggers above max number
reload issue.

The functional chang of this patch is to make gcc don't treat an memory
address reload as spill which is regression caused by r237277.

Does this explanation make sense?

Thanks.

Regards,
Jiong


Bernd


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]