Re: [RFC] Use which_alternative in preparation-statements of define_insn_and_split

2011-11-23 Thread Richard Earnshaw
On 22/11/11 19:06, Richard Henderson wrote:
> On 11/21/2011 05:38 PM, Jiangning Liu wrote:
>> But I still want to know why we don't want to support this? I don't see any
>> GCC documentation saying not allowing this usage.
> 
> Before reload, which_alternative doesn't make much sense, yet you compute it 
> anyway.  After reload, but for raw define_split, which_alternative doesn't 
> make much sense, yet you compute it anyway.
> 
> Now, either or both are fixable, but instead it seemed to me like maybe you 
> were going down the wrong path entirely.
> 
> 
> r~
> 

Plus extract_insn is a moderately expensive operation, given that we
have to scan all the alternatives in an insn to find the matching one.

I wouldn't have thought it was generally something we would want to do
on every split pattern we apply unless it is really needed.

R.



Re: [RFC] Use which_alternative in preparation-statements of define_insn_and_split

2011-11-22 Thread Richard Henderson
On 11/21/2011 05:38 PM, Jiangning Liu wrote:
> But I still want to know why we don't want to support this? I don't see any
> GCC documentation saying not allowing this usage.

Before reload, which_alternative doesn't make much sense, yet you compute it 
anyway.  After reload, but for raw define_split, which_alternative doesn't make 
much sense, yet you compute it anyway.

Now, either or both are fixable, but instead it seemed to me like maybe you 
were going down the wrong path entirely.


r~


RE: [RFC] Use which_alternative in preparation-statements of define_insn_and_split

2011-11-21 Thread Jiangning Liu


> -Original Message-
> From: Richard Henderson [mailto:r...@redhat.com]
> Sent: Tuesday, November 22, 2011 7:55 AM
> To: Jiangning Liu
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [RFC] Use which_alternative in preparation-statements of
> define_insn_and_split
> 
> On 11/20/2011 07:34 PM, Jiangning Liu wrote:
> > Hi,
> >
> > I find which_alternative can't really be used in preparation-
> statements of
> > define_insn_and_split, so can this be fixed like below?
> >
> > For example, I want to use which_alternative in the pattern below,
> >
> > (define_insn_and_split "*thumb2_movsicc_insn"
> >   [(set (match_operand:SI 0 "s_register_operand" "=r,r,r,r,r,r,r,r")
> > (if_then_else:SI
> >  (match_operator 3 "arm_comparison_operator"
> >   [(match_operand 4 "cc_register" "") (const_int 0)])
> >  (match_operand:SI 1 "arm_not_operand" "0,0,rI,K,rI,rI,K,K")
> >  (match_operand:SI 2 "arm_not_operand" "rI,K,0,0,rI,K,rI,K")))]
> >   "TARGET_THUMB2"
> >   "@
> >it\\t%D3\;mov%D3\\t%0, %2
> >it\\t%D3\;mvn%D3\\t%0, #%B2
> >it\\t%d3\;mov%d3\\t%0, %1
> >it\\t%d3\;mvn%d3\\t%0, #%B1
> >ite\\t%d3\;mov%d3\\t%0, %1\;mov%D3\\t%0, %2
> >ite\\t%d3\;mov%d3\\t%0, %1\;mvn%D3\\t%0, #%B2
> >ite\\t%d3\;mvn%d3\\t%0, #%B1\;mov%D3\\t%0, %2
> >ite\\t%d3\;mvn%d3\\t%0, #%B1\;mvn%D3\\t%0, #%B2"
> >   "&& reload_completed"
> >   [(cond_exec (match_dup 5)
> >   (set (match_dup 0) (match_dup 6)))]
> >   "
> >   {
> > if (which_alternative >= 2 && which_alternative < 4)
> >   {
> > ...
> >   }
> > else if (which_alternative >= 4)
> 
> Hmm, I guess.  It seems a bit weird.
> 
> It seems like you'd be better off *not* using define_insn_and_split,
> actually, and instead using more specific tests on the separate
> define_split than you would on the define_insn.
> 

Yes. That would be alternative solution I can accept. 

But I still want to know why we don't want to support this? I don't see any
GCC documentation saying not allowing this usage.

Or you might be thinking this change is not safe enough?

Thanks,
-Jiangning

> I don't feel strongly about it though.  I won't object if some other
> rtl maintainer wants to approve this.
> 
> 
> r~






Re: [RFC] Use which_alternative in preparation-statements of define_insn_and_split

2011-11-21 Thread Richard Henderson
On 11/20/2011 07:34 PM, Jiangning Liu wrote:
> Hi,
> 
> I find which_alternative can't really be used in preparation-statements of
> define_insn_and_split, so can this be fixed like below?
> 
> For example, I want to use which_alternative in the pattern below,
> 
> (define_insn_and_split "*thumb2_movsicc_insn"
>   [(set (match_operand:SI 0 "s_register_operand" "=r,r,r,r,r,r,r,r")
>   (if_then_else:SI
>(match_operator 3 "arm_comparison_operator"
> [(match_operand 4 "cc_register" "") (const_int 0)])
>(match_operand:SI 1 "arm_not_operand" "0,0,rI,K,rI,rI,K,K")
>(match_operand:SI 2 "arm_not_operand" "rI,K,0,0,rI,K,rI,K")))]
>   "TARGET_THUMB2"
>   "@
>it\\t%D3\;mov%D3\\t%0, %2
>it\\t%D3\;mvn%D3\\t%0, #%B2
>it\\t%d3\;mov%d3\\t%0, %1
>it\\t%d3\;mvn%d3\\t%0, #%B1
>ite\\t%d3\;mov%d3\\t%0, %1\;mov%D3\\t%0, %2
>ite\\t%d3\;mov%d3\\t%0, %1\;mvn%D3\\t%0, #%B2
>ite\\t%d3\;mvn%d3\\t%0, #%B1\;mov%D3\\t%0, %2
>ite\\t%d3\;mvn%d3\\t%0, #%B1\;mvn%D3\\t%0, #%B2"
>   "&& reload_completed"
>   [(cond_exec (match_dup 5)
> (set (match_dup 0) (match_dup 6)))]
>   "
>   {
> if (which_alternative >= 2 && which_alternative < 4)
>   {
> ...
>   }
> else if (which_alternative >= 4)

Hmm, I guess.  It seems a bit weird.

It seems like you'd be better off *not* using define_insn_and_split, actually, 
and instead using more specific tests on the separate define_split than you 
would on the define_insn.

I don't feel strongly about it though.  I won't object if some other rtl 
maintainer wants to approve this.


r~


[RFC] Use which_alternative in preparation-statements of define_insn_and_split

2011-11-20 Thread Jiangning Liu
Hi,

I find which_alternative can't really be used in preparation-statements of
define_insn_and_split, so can this be fixed like below?

For example, I want to use which_alternative in the pattern below,

(define_insn_and_split "*thumb2_movsicc_insn"
  [(set (match_operand:SI 0 "s_register_operand" "=r,r,r,r,r,r,r,r")
(if_then_else:SI
 (match_operator 3 "arm_comparison_operator"
  [(match_operand 4 "cc_register" "") (const_int 0)])
 (match_operand:SI 1 "arm_not_operand" "0,0,rI,K,rI,rI,K,K")
 (match_operand:SI 2 "arm_not_operand" "rI,K,0,0,rI,K,rI,K")))]
  "TARGET_THUMB2"
  "@
   it\\t%D3\;mov%D3\\t%0, %2
   it\\t%D3\;mvn%D3\\t%0, #%B2
   it\\t%d3\;mov%d3\\t%0, %1
   it\\t%d3\;mvn%d3\\t%0, #%B1
   ite\\t%d3\;mov%d3\\t%0, %1\;mov%D3\\t%0, %2
   ite\\t%d3\;mov%d3\\t%0, %1\;mvn%D3\\t%0, #%B2
   ite\\t%d3\;mvn%d3\\t%0, #%B1\;mov%D3\\t%0, %2
   ite\\t%d3\;mvn%d3\\t%0, #%B1\;mvn%D3\\t%0, #%B2"
  "&& reload_completed"
  [(cond_exec (match_dup 5)
  (set (match_dup 0) (match_dup 6)))]
  "
  {
if (which_alternative >= 2 && which_alternative < 4)
  {
...
  }
else if (which_alternative >= 4)
  {
...
  }
  }"
  [(set_attr "length" "6,6,6,6,10,10,10,10")
   (set_attr "conds" "use")]
)

Thanks,
-Jiangning

Patch:

diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index c94e743..df6a3df
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -3502,6 +3502,10 @@ try_split (rtx pat, rtx trial, int last)
 split_branch_probability = INTVAL (XEXP (note, 0));
   probability = split_branch_probability;

+  extract_insn (trial);
+  if (!constrain_operands (reload_completed))
+return trial;
+
   seq = split_insns (pat, trial);

   split_branch_probability = -1;