| Message ID | 000101cca7fe$83c35490$8b49fdb0$@liu@arm.com |
|---|---|
| State | New |
| Headers | show |
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~
> -----Original Message----- > From: Richard Henderson [mailto:rth@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~
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~
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.
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;
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: