diff mbox

[ia64,PR,target/57491] internal compiler error: in ia64_split_tmode -O2, quadmath

Message ID 20131107124234.GA2459@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Kirill Yukhin Nov. 7, 2013, 12:42 p.m. UTC
Hello,
I've looked into RTL after register allocation. Insn which
lead to assert is:
(insn 77 180 79 4 (set (reg/v:TF 44 loc12 [orig:359 T8 ] [359])
        (mem:TF (post_modify:DI (reg/f:DI 14 r14 [435])
                (plus:DI (reg/f:DI 14 r14 [435])
                    (reg:DI 45 loc13 [orig:342 D.3347 ] [342]))) [2 *R1_102+0 S16 A128])) 1.i:2370 131 {*movtf_intern\
al}

When trying to split it into two DI loads here:
     ia64_split_tmode_move

We check if source and destination RTX are register overlapped,
and if so, set flag that says that load adress is dead after insns:
  if (GET_CODE (operands[1]) == MEM
      && reg_overlap_mentioned_p (operands[0], operands[1]))
    ...
    dead = true;

This flags is asserted (to be unset) later in splitting of memory operand in 
ia64_split_tmode:
          case POST_MODIFY:
            gcc_assert (!reversed && !dead);

I think that the insn (although op[0] and op1[1] are overlapped by regs)
is still safe to split. And we might end up with something like this:
  (insn 272 0 0 (set (reg:DI 44 loc12)
          (mem:DI (post_inc:DI (reg/f:DI 14 r14 [435])) [2 *R1_102+0 S8 A128]))
  (insn 273 272 0 (set (reg:DI 45 loc13)
          (mem:DI (post_modify:DI (reg/f:DI 14 r14 [435])
                  (plus:DI (reg/f:DI 14 r14 [435])
                      (reg:DI 45 loc13 [orig:342 D.3347 ] [342]))) [2 *R1_102+8 S8 A64]))
  (set (reg/f:DI 14 r14 [435])
      (plus:DI (reg/f:DI 14 r14 [435])
          (const_int -8 [0xfffffffffffffff8])))

As far as I understand, second insn will update loc13 *after* ld is executed,
so value of location in r14 will be correct.

Patch in the bottom.

ChangeLog entry:
	  * config/ia64/ia64.c (ia64_split_tmode_move): Relax `dead' flag setting.

Testcase builds w/o ICE with the patch.
Bootstrapped.

Could you pls take a look?

--
Thanks, K
---
 gcc/config/ia64/ia64.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

Comments

Kirill Yukhin Nov. 12, 2013, 8:38 a.m. UTC | #1
Hello,

On 07 Nov 15:42, Kirill Yukhin wrote:
> Could you pls take a look?

Ping?

--
Thanks, K
Steve Ellcey Nov. 13, 2013, 9:56 p.m. UTC | #2
On Tue, 2013-11-12 at 11:38 +0300, Kirill Yukhin wrote:
> Hello,
> 
> On 07 Nov 15:42, Kirill Yukhin wrote:
> > Could you pls take a look?
> 
> Ping?
> 
> --
> Thanks, K

Looks OK to me, go ahead and check it in.

Steve Ellcey
sellcey@mips.com
Eric Botcazou Nov. 15, 2013, 9:51 a.m. UTC | #3
> diff --git a/gcc/config/ia64/ia64.c b/gcc/config/ia64/ia64.c
> index a128b19..446ee59 100644
> --- a/gcc/config/ia64/ia64.c
> +++ b/gcc/config/ia64/ia64.c
> @@ -1527,12 +1527,19 @@ ia64_split_tmode_move (rtx operands[])
>        && reg_overlap_mentioned_p (operands[0], operands[1]))
>      {
>        rtx base = XEXP (operands[1], 0);
> +      rtx first_write = gen_rtx_REG (DImode, REGNO (operands[0]));
>        while (GET_CODE (base) != REG)
>         base = XEXP (base, 0);
> 
>        if (REGNO (base) == REGNO (operands[0]))
> -       reversed = true;
> -      dead = true;
> +       {
> +         reversed = true;
> +         first_write = gen_rtx_REG (DImode, REGNO (operands[0]) + 1);
> +       }
> +
> +      if (GET_CODE (operands[0]) == REG
> +         && reg_overlap_mentioned_p (first_write, operands[1]))
> +       dead = true;
>      }
>    /* Another reason to do the moves in reversed order is if the first
>       element of the target register pair is also the second element of

Note that the new "GET_CODE (operands[0]) == REG" check is redundant, since 
REGNO (operands[0]) is already accessed before.  And instead of generating 
very short-lived RTL, you could use refers_to_regno_p instead.

I suspect that the problem reported by Jeff is related to the usage of "dead" 
in the REG subcase of the MEM case of ia64_split_tmode.  There is a dangling 
comment to that effect in ia64_split_tmode_move just above the block.
Kirill Yukhin Nov. 15, 2013, 10:45 a.m. UTC | #4
Hello,
> I suspect that the problem reported by Jeff is related to the usage of "dead" 
> in the REG subcase of the MEM case of ia64_split_tmode.  There is a dangling 
> comment to that effect in ia64_split_tmode_move just above the block.

We're failing here. Trying to split SET with these operands:
      operands[0] is (reg:TI 14 r14 [orig:448 *_61[_12]{lb: 1 sz: 64}.text ] [448])
      operands[1] is (mem:TI (reg/f:DI 15 r15 [447]) [3 *_61[_12]{lb: 1 sz: 64}.text+0 S16 A128])

I think that such a set (despite of intersect by r15) is valid.
This RTX is splitted by ia64_split_tmode_move into these 2 insns:
  (insn 199 0 0 (set (reg:DI 14 r14)
  	             (mem:DI (post_inc:DI (reg/f:DI 15 r15 [447])) [3 *_61[_12]{lb: 1 sz: 64}.text+0 S8 A128])) -1
	             (nil))
  (insn 200 199 0 (set (reg:DI 15 r15)
  	               (mem:DI (post_dec:DI (reg/f:DI 15 r15 [447])) [3 *_61[_12]{lb: 1 sz: 64}.text+8 S8 A64])) -1
		       (nil))

Although r15 post_dec in second insn seems to be useless code seems to be valid for me.
What do you guys think?

--
Thanks, K
Eric Botcazou Nov. 15, 2013, 11:34 a.m. UTC | #5
> We're failing here. Trying to split SET with these operands:
>       operands[0] is (reg:TI 14 r14 [orig:448 *_61[_12]{lb: 1 sz: 64}.text ]
> [448]) operands[1] is (mem:TI (reg/f:DI 15 r15 [447]) [3 *_61[_12]{lb: 1
> sz: 64}.text+0 S16 A128])
> 
> I think that such a set (despite of intersect by r15) is valid.

Yes, that's the purpose of the block in ia64_split_tmode_move.

> This RTX is splitted by ia64_split_tmode_move into these 2 insns:
>   (insn 199 0 0 (set (reg:DI 14 r14)
>   	             (mem:DI (post_inc:DI (reg/f:DI 15 r15 [447])) [3
> *_61[_12]{lb: 1 sz: 64}.text+0 S8 A128])) -1 (nil))
>   (insn 200 199 0 (set (reg:DI 15 r15)
>   	               (mem:DI (post_dec:DI (reg/f:DI 15 r15 [447])) [3
> *_61[_12]{lb: 1 sz: 64}.text+8 S8 A64])) -1 (nil))
> 
> Although r15 post_dec in second insn seems to be useless code seems to be
> valid for me. What do you guys think?

But the post_dec will clobber r15, won't it?
diff mbox

Patch

diff --git a/gcc/config/ia64/ia64.c b/gcc/config/ia64/ia64.c
index a128b19..446ee59 100644
--- a/gcc/config/ia64/ia64.c
+++ b/gcc/config/ia64/ia64.c
@@ -1527,12 +1527,19 @@  ia64_split_tmode_move (rtx operands[])
       && reg_overlap_mentioned_p (operands[0], operands[1]))
     {
       rtx base = XEXP (operands[1], 0);
+      rtx first_write = gen_rtx_REG (DImode, REGNO (operands[0]));
       while (GET_CODE (base) != REG)
        base = XEXP (base, 0);

       if (REGNO (base) == REGNO (operands[0]))
-       reversed = true;
-      dead = true;
+       {
+         reversed = true;
+         first_write = gen_rtx_REG (DImode, REGNO (operands[0]) + 1);
+       }
+
+      if (GET_CODE (operands[0]) == REG
+         && reg_overlap_mentioned_p (first_write, operands[1]))
+       dead = true;
     }
   /* Another reason to do the moves in reversed order is if the first
      element of the target register pair is also the second element of