RFA: Fix mark_target_live_regs to take COND_EXEC into account

RFA: Fix mark_target_live_regs to take COND_EXEC into account

[Joern Rennecke]

[-- Attachment #1: Type: text/plain, Size: 537 bytes --]

We found that

std::basic_string<char, std::char_traits<char>, std::allocator<char>
::copy(char*, unsigned long, unsigned long) const

got miscompiled for ARC because reorg thought that all call-clobbered
registers were dead after a conditional call.
I can't reproduce the test case with current trunk + ARC port, but I reckon
this is just due to the fickleness of the register allocator.  Chances are,
every other version, some other obscure function is miscompiled without
this patch.

bootstrapped on i686-pc-linux-gnu.

OK to apply?

[-- Attachment #2: tmp --]
[-- Type: text/plain, Size: 1405 bytes --]

2013-04-29 Claudiu Zissulescu <claziss@synopsys.com>

	* resource.c (mark_target_live_regs): Compute resources taking
	into account if a call is predicated or not.

diff --git a/gcc/resource.c b/gcc/resource.c
Index: resource.c
===================================================================
--- resource.c	(revision 202295)
+++ resource.c	(working copy)
@@ -994,11 +994,18 @@ mark_target_live_regs (rtx insns, rtx ta
 
 	  if (CALL_P (real_insn))
 	    {
-	      /* CALL clobbers all call-used regs that aren't fixed except
-		 sp, ap, and fp.  Do this before setting the result of the
-		 call live.  */
-	      AND_COMPL_HARD_REG_SET (current_live_regs,
-				      regs_invalidated_by_call);
+	      /* Values in call-clobbered registers survive a COND_EXEC CALL
+		 if that is not executed; this matters for resoure use because
+		 they may be used by a complementarily (or more strictly)
+		 predicated instruction, or if the CALL is NORETURN.  */
+	      if (GET_CODE (PATTERN (real_insn)) != COND_EXEC)
+		{
+		  /* CALL clobbers all call-used regs that aren't fixed except
+		     sp, ap, and fp.  Do this before setting the result of the
+		     call live.  */
+		  AND_COMPL_HARD_REG_SET (current_live_regs,
+					  regs_invalidated_by_call);
+		}
 
 	      /* A CALL_INSN sets any global register live, since it may
 		 have been modified by the call.  */

Re: RFA: Fix mark_target_live_regs to take COND_EXEC into account

[Steven Bosscher]

On Fri, Sep 6, 2013 at 12:20 PM, Joern Rennecke wrote:
> We found that
>
> std::basic_string<char, std::char_traits<char>, std::allocator<char>
> ::copy(char*, unsigned long, unsigned long) const
>
> got miscompiled for ARC because reorg thought that all call-clobbered
> registers were dead after a conditional call.

Hmm, interesting to have a target with both predication and delay slots...

> I can't reproduce the test case with current trunk + ARC port, but I reckon
> this is just due to the fickleness of the register allocator.  Chances are,
> every other version, some other obscure function is miscompiled without
> this patch.
>
> bootstrapped on i686-pc-linux-gnu.

That's not a very good test, i686 doesn't use this code ;-)

> OK to apply?

Yes, the patch is OK.

However, I wouldn't be surprised if using COND_EXECs together with
delay slots has more hidden problems. Browsing through resource.c, the
"if (jump_insn)" block at lines 1095:1119 jumps out as another place
you should probably look into.

Ciao!
Steven