RE: [PATCH 4/7] [ARC] [LRA] Avoid emitting COND_EXEC during expand.

[Claudiu Zissulescu]

> This seems fine, your description "does not always work." is a bit
> of a tease :) it would be nice to know _why_ it doesn't work, or at
> least a description of what problem you're seeing.

Committed with additional comments. Thank you, 
Claudiu

RE: [PATCH 4/7] [ARC] [LRA] Avoid emitting COND_EXEC during expand.

[Claudiu Zissulescu]

> This seems fine, your description "does not always work." is a bit
> of a tease :) it would be nice to know _why_ it doesn't work, or at
> least a description of what problem you're seeing.
> 
As far as I can see, LRA doesn't handle very well the conditional execution 
patterns, as it expects conditional execution to happen after this step. Thus, 
some of those instructions are marked dead and removed later on.

> Also we seem to be missing a test, would it be possible to find one?
> If not then I guess we live without, but we should note that in the
> commit message.

This error is found by executing dg.exp testsuite with our port and -mlra 
option on.  As we speak, I am on the last 100m of testing our port having the 
LRA on. This bug being found like that.

I'll add this discussion to the commit message body.

Thank you,
Claudiu

Re: [PATCH 4/7] [ARC] [LRA] Avoid emitting COND_EXEC during expand.

[Andrew Burgess]

* Claudiu Zissulescu  [2017-06-01 15:34:54 
+0200]:

> Emmitting COND_EXEC rtxes during expand does not always work.
> 
> gcc/
> 2017-01-10  Claudiu Zissulescu  
> 
>   * config/arc/arc.md (clzsi2): Expand to an arc_clzsi2 instruction
>   that also clobbers the CC register. The old expand code is moved
>   to ...
>   (*arc_clzsi2): ... here.
>   (ctzsi2): Expand to an arc_ctzsi2 instruction that also clobbers
>   the CC register. The old expand code is moved to ...
>   (arc_ctzsi2): ... here.

This seems fine, your description "does not always work." is a bit
of a tease :) it would be nice to know _why_ it doesn't work, or at
least a description of what problem you're seeing.

Also we seem to be missing a test, would it be possible to find one?
If not then I guess we live without, but we should note that in the
commit message.

Thanks,
Andrew




> ---
>  gcc/config/arc/arc.md | 41 ++---
>  1 file changed, 34 insertions(+), 7 deletions(-)
> 
> diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md
> index 39bcc26..928feb1 100644
> --- a/gcc/config/arc/arc.md
> +++ b/gcc/config/arc/arc.md
> @@ -4533,9 +4533,21 @@
> (set_attr "type" "two_cycle_core,two_cycle_core")])
>  
>  (define_expand "clzsi2"
> -  [(set (match_operand:SI 0 "dest_reg_operand" "")
> - (clz:SI (match_operand:SI 1 "register_operand" "")))]
> +  [(parallel
> +[(set (match_operand:SI 0 "register_operand" "")
> +   (clz:SI (match_operand:SI 1 "register_operand" "")))
> + (clobber (match_dup 2))])]
> +  "TARGET_NORM"
> +  "operands[2] = gen_rtx_REG (CC_ZNmode, CC_REG);")
> +
> +(define_insn_and_split "*arc_clzsi2"
> +  [(set (match_operand:SI 0 "register_operand" "=r")
> + (clz:SI (match_operand:SI 1 "register_operand" "r")))
> +   (clobber (reg:CC_ZN CC_REG))]
>"TARGET_NORM"
> +  "#"
> +  "reload_completed"
> +  [(const_int 0)]
>  {
>emit_insn (gen_norm_f (operands[0], operands[1]));
>emit_insn
> @@ -4552,9 +4564,23 @@
>  })
>  
>  (define_expand "ctzsi2"
> -  [(set (match_operand:SI 0 "register_operand" "")
> - (ctz:SI (match_operand:SI 1 "register_operand" "")))]
> +  [(match_operand:SI 0 "register_operand" "")
> +   (match_operand:SI 1 "register_operand" "")]
>"TARGET_NORM"
> +  "
> +  emit_insn (gen_arc_ctzsi2 (operands[0], operands[1]));
> +  DONE;
> +")
> +
> +(define_insn_and_split "arc_ctzsi2"
> +  [(set (match_operand:SI 0 "register_operand" "=r")
> + (ctz:SI (match_operand:SI 1 "register_operand" "r")))
> +   (clobber (reg:CC_ZN CC_REG))
> +   (clobber (match_scratch:SI 2 "=&r"))]
> +  "TARGET_NORM"
> +  "#"
> +  "reload_completed"
> +  [(const_int 0)]
>  {
>rtx temp = operands[0];
>  
> @@ -4562,10 +4588,10 @@
>|| (REGNO (temp) < FIRST_PSEUDO_REGISTER
> && !TEST_HARD_REG_BIT (reg_class_contents[GENERAL_REGS],
>REGNO (temp
> -temp = gen_reg_rtx (SImode);
> +temp = operands[2];
>emit_insn (gen_addsi3 (temp, operands[1], constm1_rtx));
>emit_insn (gen_bic_f_zn (temp, temp, operands[1]));
> -  emit_insn (gen_clrsbsi2 (temp, temp));
> +  emit_insn (gen_clrsbsi2 (operands[0], temp));
>emit_insn
>  (gen_rtx_COND_EXEC
>(VOIDmode,
> @@ -4575,7 +4601,8 @@
>  (gen_rtx_COND_EXEC
>(VOIDmode,
> gen_rtx_GE (VOIDmode, gen_rtx_REG (CC_ZNmode, CC_REG), const0_rtx),
> -   gen_rtx_SET (operands[0], gen_rtx_MINUS (SImode, GEN_INT (31), 
> temp;
> +   gen_rtx_SET (operands[0], gen_rtx_MINUS (SImode, GEN_INT (31),
> + operands[0];
>DONE;
>  })
>  
> -- 
> 1.9.1
>