Re: [PATCH 3/6] [ARC] Add BI/BIH instruction support.

[claziss]

Committed with your feedback in. Thank you,
Claudiu

Re: [PATCH 3/6] [ARC] Add BI/BIH instruction support.

[claziss]

Thank you all for your review. The patch is pushed with your input in.

//Claudiu

RE: [PATCH 3/6] [ARC] Add BI/BIH instruction support.

[Claudiu Zissulescu]

Hi,

> 
> This removes the compact-casesi as an option for earlier ARC, right?
> Was there a reason why that had to be done?
> 

The compact-casesi was only designed for ARCv1 types of CPUs. Unfortunately, it 
was error prone leading to all kinds of runtime and compile time errors. Fixing 
it will nullify most of the advantages of using this format. Hence, I've remove 
it, leaving the standard implementation for all CPUS. 
The BI/BIH are two new instructions for ARCv2, and they should be friendly with 
the cache, but I couldn't prove that, hence they are guarded by an option.

Please let me know if you want to have compact-casesi back in the compiler.

Thanks,
Claudiu

Re: [PATCH 3/6] [ARC] Add BI/BIH instruction support.

[Sandra Loosemore]

On 10/10/2018 02:00 AM, Claudiu Zissulescu wrote:

Use BI/BIH instruction to implement casesi pattern. Only ARC V2.


Very minor nit in the documentation part of this patch:


diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 802cc642453..454587310c8 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -650,7 +650,7 @@ Objective-C and Objective-C++ Dialects}.
  -mmixed-code  -mq-class  -mRcq  -mRcw  -msize-level=@var{level} @gol
  -mtune=@var{cpu}  -mmultcost=@var{num} @gol
  -munalign-prob-threshold=@var{probability}  -mmpy-option=@var{multo} @gol
--mdiv-rem  -mcode-density  -mll64  -mfpu=@var{fpu} -mrf16}
+-mdiv-rem  -mcode-density  -mll64  -mfpu=@var{fpu} -mrf16 -mbranch-index}
  
  @emph{ARM Options}

  @gccoptlist{-mapcs-frame  -mno-apcs-frame @gol


Please consistently use 2 spaces between options listed on the same line 
in these option summary tables.  It makes the output a little easier to 
read.


-Sandra

Re: [PATCH 3/6] [ARC] Add BI/BIH instruction support.

[Andrew Burgess]

* Claudiu Zissulescu  [2018-10-10 11:00:13 +0300]:

> Use BI/BIH instruction to implement casesi pattern. Only ARC V2.

This removes the compact-casesi as an option for earlier ARC, right?
Was there a reason why that had to be done?

> 
> gcc/
> 2018-03-21  Claudiu Zissulescu  
> 
>   * config/arc/arc.c (arc_override_options): Remove
>   TARGET_COMPACT_CASESI.
>   * config/arc/arc.h (ASM_OUTPUT_ADDR_DIFF_ELT): Update.
>   (CASE_VECTOR_MODE): Likewise.
>   (CASE_VECTOR_PC_RELATIVE): Likewise.
>   (CASE_VECTOR_SHORTEN_MODE): Likewise.
>   (CASE_VECTOR_SHORTEN_MODE1): Delete.
>   (ADDR_VEC_ALIGN): Update.
>   (ASM_OUTPUT_CASE_LABEL): Undefine.
>   (ASM_OUTPUT_BEFORE_CASE_LABEL): Undefine.
>   (TARGET_BI_BIH): Define.
>   (DEFAULT_BRANCH_INDEX): Likewise.
>   * config/arc/arc.md (casesi): Rework to accept BI/BIH
>   instructions, remove compact_casesi use case.
>   (casesi_compact_jump): Remove.
>   (casesi_dispatch): New pattern.
>   * config/arc/arc.opt: Add mbranch-index option. Deprecate
>   compact_casesi option.
>   * doc/invoke.texi: Document mbranch-index option.

I guess if you feel that dropping compact-casesi support for earlier
targets is appropriate, then that's fine.  There's some formatting
issues I point out below.  But otherwise seems reasonable.

Thanks,
Andrew

> 
> gcc/testsuite
> Claudiu Zissulescu  
> 
>   * gcc.target/arc/jumptable.c: New test.
> ---
>  gcc/config/arc/arc.c |  19 --
>  gcc/config/arc/arc.h | 106 ++-
>  gcc/config/arc/arc.md| 218 +++
>  gcc/config/arc/arc.opt   |   6 +-
>  gcc/doc/invoke.texi  |   9 +-
>  gcc/testsuite/gcc.target/arc/jumptable.c |  34 
>  6 files changed, 171 insertions(+), 221 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/arc/jumptable.c
> 
> diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
> index 56f566795ff..18dd0de6af7 100644
> --- a/gcc/config/arc/arc.c
> +++ b/gcc/config/arc/arc.c
> @@ -1291,33 +1291,14 @@ arc_override_options (void)
>if (arc_size_opt_level == 3)
>  optimize_size = 1;
>  
> -  /* Compact casesi is not a valid option for ARCv2 family.  */
> -  if (TARGET_V2)
> -{
> -  if (TARGET_COMPACT_CASESI)
> - {
> -   warning (OPT_mcompact_casesi,
> -"compact-casesi is not applicable to ARCv2");
> -   TARGET_COMPACT_CASESI = 0;
> - }
> -}
> -  else if (optimize_size == 1
> -&& !global_options_set.x_TARGET_COMPACT_CASESI)
> -TARGET_COMPACT_CASESI = 1;
> -
>if (flag_pic)
>  target_flags |= MASK_NO_SDATA_SET;
>  
>if (flag_no_common == 255)
>  flag_no_common = !TARGET_NO_SDATA_SET;
>  
> -  /* TARGET_COMPACT_CASESI needs the "q" register class.  */
>if (TARGET_MIXED_CODE)
>  TARGET_Q_CLASS = 1;
> -  if (!TARGET_Q_CLASS)
> -TARGET_COMPACT_CASESI = 0;
> -  if (TARGET_COMPACT_CASESI)
> -TARGET_CASE_VECTOR_PC_RELATIVE = 1;
>  
>/* Check for small data option */
>if (!global_options_set.x_g_switch_value && !TARGET_NO_SDATA_SET)
> diff --git a/gcc/config/arc/arc.h b/gcc/config/arc/arc.h
> index dd78a6bbbd1..cb48b85d6e7 100644
> --- a/gcc/config/arc/arc.h
> +++ b/gcc/config/arc/arc.h
> @@ -1264,25 +1264,39 @@ do {  
> \
>  } while (0)
>  
>  /* This is how to output an element of a case-vector that is relative.  */
> -#define ASM_OUTPUT_ADDR_DIFF_ELT(FILE, BODY, VALUE, REL) \
> -do { \
> -  char label[30];\
> -  ASM_GENERATE_INTERNAL_LABEL (label, "L", VALUE);   \
> -  switch (GET_MODE (BODY))   \
> -{\
> -case E_QImode: fprintf (FILE, "\t.byte "); break;\
> -case E_HImode: fprintf (FILE, "\t.hword "); break;   \
> -case E_SImode: fprintf (FILE, "\t.word "); break;\
> -default: gcc_unreachable (); \
> -}\
> -  assemble_name (FILE, label);   \
> -  fprintf (FILE, "-");   \
> -  ASM_GENERATE_INTERNAL_LABEL (label, "L", REL); \
> -  assemble_name (FILE, label);   \
> -  if (TARGET_COMPACT_CASESI) \
> -fprintf (FILE, " + %d", 4 + arc_get_unalign ()); \
> -  fprintf(FILE, "\n");  \
> -} while (0)
> +#define ASM_OUTPUT_ADDR_DIFF_ELT(FILE, BODY, VALUE, REL) \
> +  do {   \
> +char label[30];  \
> +ASM_GENERATE_INTERNAL_LABEL (label, "L", VALUE); \
> +if (!TARGET_BI_BIH)  \
> +  {