ifcvt.c question.

[Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>]

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

Hi,

While experimenting to see if some of the bits in the ARM backend which
do predicated execution can be removed in favour of the more generic
bits in ifcvt.c. I attempted to turn on conditional calls using the
predicable attribute on the ARM port.


I've run into an ICE in the ifcvt pass for the following testcase. 

typedef int SItype __attribute__ ((mode (SI)));
typedef int DItype __attribute__ ((mode (DI)));

SItype
__mulvsi3 (SItype a, SItype b)
{
  const DItype w = (DItype) a * (DItype) b;

  if ((SItype) (w >> (4 * 8)) != (SItype) w >> ((4 * 8) - 1))
    abort ();

  return w;
}

test.c:14:1: internal compiler error: in merge_if_block, at ifcvt.c:2968
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.

The ICE is because the following assert fails.

      /* We cannot merge the JOIN.  */
      /* The outgoing edge for the current COMBO block should already
be correct.  Verify this.  */
      gcc_assert (single_succ_p (combo_bb)
  && single_succ (combo_bb) == join_bb);



Here's what ifcvt does before the crash . 

IF-THEN block found, pass 1, start block 2 [insn 5], then 3 [14], join 1
[-1]
rescanning insn with uid = 15.
deleting insn with uid = 15.
1 insn was converted to conditional execution.
merging block 3 into block 2
deleting insn with uid = 13.
deleting insn with uid = 14.
changing bb of uid 15
  from 3 to 2


The insns deleted are : 

(jump_insn 13 12 14 2 test.c:9 (set (pc)
        (if_then_else (eq (reg:CC 24 cc)
                (const_int 0 [0x0]))
            (return)
            (pc))) 258 {*cond_return} (expr_list:REG_DEAD (reg:CC 24 cc)
        (expr_list:REG_BR_PROB (const_int 9996 [0x270c])
            (nil))))

(note 14 13 15 3 [bb 3] NOTE_INSN_BASIC_BLOCK)

insn 15 whose bb is changed from 3 to 2 is the call to abort. 


(call_insn 15 14 16 3 test.c:10 (parallel [
            (call (mem:SI (symbol_ref:SI ("abort") [flags 0x41]
<function_decl 0x2aaaaab7be00 abort>) [0 S4 A32])
                (const_int 0 [0x0]))
            (use (const_int 0 [0x0]))
            (clobber (reg:SI 14 lr))
        ]) 251 {*call_symbol} (expr_list:REG_NORETURN (const_int 0
[0x0])
        (expr_list:REG_EH_REGION (const_int 0 [0x0])
            (nil)))
    (nil))

In this case before if-conversion you have a basic block structure of
the following form. 

   [2] (cond_return is the last instruction)
    |
   [3] (call abort)  (REG_EH_NO_RETURN)
    |
   [1] - Exit block 


ifcvt then goes ahead and converts the call to abort into a conditional
call and should ideally convert the conditional return as a normal
return i.e. by moving the last instruction in [2] into [3] as an
unconditional return. It does the first part correctly which is
converting the call to a conditional call but incorrectly removes the
unconditional return. 

Note however that thread_prologue_and_epilogue inserts an extra return
instruction in basic block 4 but this gets removed by cfg_cleanup the
next time it is run before dse2 because bb4 is unreachable. The ARM
backend is correct in generating a conditional return in this case,
because it is capable of generating a load multiple instruction that
restores the PC as well as the saved integer registers. 

I wonder if the best way to fix this is to teach ifcvt.c to handle
conditional returns.  Should I be fixing this inside ifcvt.c by handling
conditional returns specially or should I be looking elsewhere ? 

I've attached the dumps and the original patch to turn on the predicable
attribute for the ARM backend with this email. 

Thanks in advance for the answers. 

cheers
Ramana


Attachments 

1. Patch to turn on predicable attributes on all the ARM call patterns. 
2. Dumps of the testcase that are relevant, test.c.*.peephole2,
test.c.*.dse2, test.c.*.ce3. (dumps.tar.bz2)
3. Testcase test.c 


-- 
Ramana Radhakrishnan
GNU Tools 
ARM Ltd.

[-- Attachment #2: calls_predicable.patch --]
[-- Type: text/x-patch, Size: 2353 bytes --]

Index: arm.md
===================================================================
--- arm.md	(revision 150565)
+++ arm.md	(working copy)
@@ -4860,7 +4860,8 @@ (define_insn "*arm_movdi"
   [(set_attr "length" "8,12,16,8,8")
    (set_attr "type" "*,*,*,load2,store2")
    (set_attr "pool_range" "*,*,*,1020,*")
-   (set_attr "neg_pool_range" "*,*,*,1008,*")]
+   (set_attr "neg_pool_range" "*,*,*,1008,*")
+   (set_attr "predicable" "yes")]
 )
 
 (define_split
@@ -8446,7 +8447,8 @@ (define_insn "*call_value_reg_armv5"
    (clobber (reg:SI LR_REGNUM))]
   "TARGET_ARM && arm_arch5"
   "blx%?\\t%1"
-  [(set_attr "type" "call")]
+  [(set_attr "type" "call")
+   (set_attr "predicable" "yes")]
 )
 
 (define_insn "*call_value_reg_arm"
@@ -8460,7 +8462,8 @@ (define_insn "*call_value_reg_arm"
   return output_call (&operands[1]);
   "
   [(set_attr "length" "12")
-   (set_attr "type" "call")]
+   (set_attr "type" "call")
+   (set_attr "predicable" "yes")]
 )
 
 (define_insn "*call_value_mem"
@@ -8474,7 +8477,8 @@ (define_insn "*call_value_mem"
   return output_call_mem (&operands[1]);
   "
   [(set_attr "length" "12")
-   (set_attr "type" "call")]
+   (set_attr "type" "call")
+   (set_attr "predicable" "yes")]
 )
 
 (define_insn "*call_value_reg_thumb1_v5"
@@ -8525,7 +8529,8 @@ (define_insn "*call_symbol"
   {
     return NEED_PLT_RELOC ? \"bl%?\\t%a0(PLT)\" : \"bl%?\\t%a0\";
   }"
-  [(set_attr "type" "call")]
+  [(set_attr "type" "call")
+   (set_attr "predicable" "yes")]
 )
 
 (define_insn "*call_value_symbol"
@@ -8541,7 +8546,8 @@ (define_insn "*call_value_symbol"
   {
     return NEED_PLT_RELOC ? \"bl%?\\t%a1(PLT)\" : \"bl%?\\t%a1\";
   }"
-  [(set_attr "type" "call")]
+  [(set_attr "type" "call")
+   (set_attr "predicable" "yes")]
 )
 
 (define_insn "*call_insn"
@@ -8608,7 +8614,8 @@ (define_insn "*sibcall_insn"
   "*
   return NEED_PLT_RELOC ? \"b%?\\t%a0(PLT)\" : \"b%?\\t%a0\";
   "
-  [(set_attr "type" "call")]
+  [(set_attr "type" "call")
+   (set_attr "predicable" "yes")]
 )
 
 (define_insn "*sibcall_value_insn"
@@ -8621,7 +8628,8 @@ (define_insn "*sibcall_value_insn"
   "*
   return NEED_PLT_RELOC ? \"b%?\\t%a1(PLT)\" : \"b%?\\t%a1\";
   "
-  [(set_attr "type" "call")]
+  [(set_attr "type" "call")
+   (set_attr "predicable" "yes")]
 )
 
 ;; Often the return insn will be the same as loading from memory, so set attr

[-- Attachment #3: dumps.tar.bz2 --]
[-- Type: application/x-bzip-compressed-tar, Size: 4694 bytes --]

[-- Attachment #4: test.c --]
[-- Type: text/x-csrc, Size: 292 bytes --]


typedef int SItype __attribute__ ((mode (SI)));
typedef int DItype __attribute__ ((mode (DI)));
void abort (void);
SItype
__mulvsi3 (SItype a, SItype b)
{
  const DItype w = (DItype) a * (DItype) b;

  if ((SItype) (w >> (4 * 8)) != (SItype) w >> ((4 * 8) - 1))
    abort ();

  return w;
}