This is the mail archive of the gcc@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]

x86 reload bug in merge_assigned_reloads


I am looking at a testcase extracted by Richard Sandiford from the gdb mips
simulator.  This testcase is miscompiled at -O because of a reload problem.

We have an insn that needs 6 reloads.
(insn 19 18 21 0 (nil) (parallel [
            (set (mem:DI (plus:SI (plus:SI (mult:SI (reg/v:SI 58)
				(const_int 8 [0x8]))
                            (reg/v/f:SI 61))
			(const_int 32 [0x20])) [0 S8 A64])
		(plus:DI (reg:DI 0 rax [62])
                    (mem:DI (plus:SI (plus:SI (mult:SI (reg/v:SI 59)
                                    (const_int 8 [0x8]))
				(reg/v/f:SI 61))
                            (const_int 32 [0x20])) [0 S8 A64])))
            (clobber (reg:CC 17 flags))
	]) 185 {*adddi3_1} (insn_list 16 (insn_list 4 (insn_list 6 (insn_list 3\ (nil)))))
    (expr_list:REG_DEAD (reg:DI 0 rax [62])
	(expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))))

At the start of merge_assigned_reloads, the first four reloads are
Reload 0: reload_in (SI) = (reg/v:SI 58)
	INDEX_REGS, RELOAD_FOR_OUTPUT_ADDRESS (opnum = 0)
	reload_in_reg: (reg/v:SI 58)
	reload_reg_rtx: (reg:SI 2 ecx)
Reload 1: reload_in (SI) = (reg/v/f:SI 61)
	GENERAL_REGS, RELOAD_FOR_OUTPUT_ADDRESS (opnum = 0)
        reload_in_reg: (reg/v/f:SI 61)
	reload_reg_rtx: (reg:SI 4 esi)
Reload 2: reload_in (SI) = (reg/v:SI 59)
	INDEX_REGS, RELOAD_FOR_OPERAND_ADDRESS (opnum = 2)
	reload_in_reg: (reg/v:SI 59)
	reload_reg_rtx: (reg:SI 2 ecx)
Reload 3: reload_in (SI) = (reg/v/f:SI 61)
	GENERAL_REGS, RELOAD_FOR_OPERAND_ADDRESS (opnum = 2)
	reload_in_reg: (reg/v/f:SI 61)
	reload_reg_rtx: (reg:SI 4 esi)
...

merge_assigned_reloads decides to merge reload 1 with reload 3 which is OK.
This converts reload 1 into a RELOAD_OTHER.  We then call the function
reg_overlap_mentioned_for_reload_p to check to see if any other reloads
must also be converted to RELOAD_OTHER.  This function notices that both
reg 58 and reg 61 are pseudos with reg_equiv_memory_loc nonzero, so it
assumes both are MEMs and thus conflict.  Likewise for reg 59 and reg 61.
So both reload 0 and reload 2 are also converted to RELOAD_OTHER.  Now we
have
Reload 0: reload_in (SI) = (reg/v:SI 58)
	INDEX_REGS, RELOAD_OTHER (opnum = 0)
	reload_in_reg: (reg/v:SI 58)
	reload_reg_rtx: (reg:SI 2 ecx)
...
Reload 2: reload_in (SI) = (reg/v:SI 59)
	INDEX_REGS, RELOAD_OTHER (opnum = 2)
	reload_in_reg: (reg/v:SI 59)
	reload_reg_rtx: (reg:SI 2 ecx)
...
and now we have a problem, because we can't have two reloads using the same
reload register that are both RELOAD_OTHER.  One clobbers the other, and the
program fails at run time.

I don't see any obvious good option here.

We could perhaps extend reload_overlap_mentioned_for_reload_p to handle the
case of two MEMs that refer to stack slots, and check to see if the stack
slots overlap.  This would prevent reload 0 and reload 2 from being converted
to RELOAD_OTHER.  It isn't clear what other side effects would result from
this.

We could perhaps add code to try to detect this conflict before we decide
to merge reload 1 with reload 3, however, it isn't clear how to do that.
I think we would have to add another loop to check for this case.  It would
iterate through all reloads, call reg_overlap_mentioned_for_reload_p to check
for which other reloads would get converted to RELOAD_OTHER, and if any two
of those reloads conflict with each other, then we can't merge the original
two reloads. 

If anyone else has other ideas, I'd like to hear them.

Here is a simplified testcase.

void foo (void) {}
void g () {}
void f (int x, int y, int c, long long *s)
{
  foo ();
  asm volatile ("" ::: "edi", "esi");
  s[4 + x] = s[4 + y] + c;
  if (c)
    g (x, y, c, s);
}

int main ()
{
  long long s[11];
  s[5] = 1;
  s[4] = 0;
  f (0, 1, 2, s);
  if (s[4] != 3)
    abort ();
}


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