On Mon, 2021-11-08 at 10:30 +0800, Chenghua Xu wrote:
> This patch does not arrive at  mail list. Send as an attachment in a 
> compressed format.

I think .patch.gz is perferred instead of .tar.gz.

And is it possible to seperate this into multiple commits?  For example
the whole "-march=native" support can be in a seperate commit.  It will
be easier to review those changes one-by-one.

> --- /dev/null
> +++ b/gcc/config/loongarch/linux.h
/* snip */
> +  /* Integer ABI  */
> +  #if DEFAULT_ABI_INT == ABI_LP64
> +    #define INT_ABI_SUFFIX "lib64"
> +  #endif

"INT_ABI_SUFFIX" should be renamed to INT_ABI_LIBDIR or something. 
"lib64" is not a "suffix".

> --- /dev/null
> +++ b/gcc/config/loongarch/loongarch-opts.c
/* snip */

> +  /* 5.  Check integer ABI-ISA for conflicts.  */
> +  switch (*isa_int)
> +    {
> +    case ISA_LA64:
> +      if (*abi_int != ABI_LP64) goto error_int_abi;
> +      break;

/* snip */

> +      switch (*isa_float)
> +       {
> +       case ISA_SOFT_FLOAT:
> +         if (*abi_float != ABI_SOFT_FLOAT) goto error_float_abi;
> +         break;
> +
> +       case ISA_SINGLE_FLOAT:
> +         if (*abi_float != ABI_SINGLE_FLOAT) goto error_float_abi;
> +         break;
> +
> +       case ISA_DOUBLE_FLOAT:
> +         if (*abi_float != ABI_DOUBLE_FLOAT) goto error_float_abi;
> +         break;
> 

The goto statements should be in a new line (coding style).

> --- /dev/null
> +++ b/gcc/config/loongarch/gnu-user.h
/* snip */

> +#define GLIBC_DYNAMIC_LINKER_LP64 "/lib64/ld.so.1"

It is "ld-linux-loongarch-lp64d.so.x" in the latest ELF psABI.  "x" is
now 0 but we have an ongoing discussion to make it 1.

> +++ b/gcc/config/loongarch/sync.md
/* snip */

> +(define_insn "atomic_cas_value_strong<mode>"
> +  [(set (match_operand:GPR 0 "register_operand" "=&r")
> +       (match_operand:GPR 1 "memory_operand" "+ZC"))
> +   (set (match_dup 1)
> +       (unspec_volatile:GPR [(match_operand:GPR 2 "reg_or_0_operand" "rJ")
> +                             (match_operand:GPR 3 "reg_or_0_operand" "rJ")
> +                             (match_operand:SI 4 "const_int_operand")  ;; 
> mod_s
> +                             (match_operand:SI 5 "const_int_operand")] ;; 
> mod_f
> +        UNSPEC_COMPARE_AND_SWAP))
> +   (clobber (match_scratch:GPR 6 "=&r"))]
> +  ""
> +{
> +  static char buff[256] = {0};

> +  buff[0] = '\0';
> +  sprintf (buff + strlen (buff), "%%G5\\n\\t");
> +  sprintf (buff + strlen (buff), "1:\\n\\t");
> +  sprintf (buff + strlen (buff), "ll.<amo>\\t%%0,%%1\\n\\t");
> +  sprintf (buff + strlen (buff), "bne\\t%%0,%%z2,2f\\n\\t");
> +  sprintf (buff + strlen (buff), "or%%i3\\t%%6,$zero,%%3\\n\\t");
> +  sprintf (buff + strlen (buff), "sc.<amo>\\t%%6,%%1\\n\\t");
> +  sprintf (buff + strlen (buff), "beq\\t$zero,%%6,1b\\n\\t");
> +  sprintf (buff + strlen (buff), "b\\t3f\\n\\t");
> +  sprintf (buff + strlen (buff), "2:\\n\\t");
> +  sprintf (buff + strlen (buff), "dbar\\t0x700\\n\\t");
> +  sprintf (buff + strlen (buff), "3:\\n\\t");
> +
> +  return buff;
> +}

These "cascading" sprintf/strlen looks stupid. It can be simply:

  return "%G5\\n\\t"
         "1:\\n\\t"
         "ll.<amo>\\t%0,%1\\n\\t"
         ...
         "3:\\n\\t";

The compiler will concatenate the string literals so there will be no
runtime overhead.

And there should be some comment to explain this LL/SC loop and dbar
workaround IMO.

Likewise for other atomic LL/SC expansions.
-- 
Xi Ruoyao <xry...@mengyan1223.wang>
School of Aerospace Science and Technology, Xidian University

Reply via email to