Re: [PATCH v8 04/12] LoongArch Port: Machine description files.

[程璐璐]

Hi,  Richard:

  Thanks for your review.
  We will revise it as soon as possible and submit it in the next version.

在 2022/3/7 上午12:16, Richard Sandiford 写道:


Hi,

Some comments below, but otherwise it looks good to me.

xucheng...@loongson.cn writes:

[…]
+(define_memory_constraint "k"
+  "A memory operand whose address is formed by a base register and (optionally 
scaled)
+   index register."
+  (and (match_code "mem")
+   (not (match_test "loongarch_14bit_shifted_offset_address_p (XEXP (op, 0), 
mode)"))
+   (not (match_test "loongarch_12bit_offset_address_p (XEXP (op, 0), 
mode)"

It's not really safe to test MEM addresses using only negative conditions.
There needs to be a positive condition too, even if it's only:

(match_test "memory_address_addr_space_p (GET_MODE (op), XEXP (op, 0),
 MEM_ADDR_SPACE (op))")))

(from common.md).


[…]
+(define_constraint "v"
+  "A nsigned 64-bit constant and low 44-bit is zero (for logic instructions)."

typo


[…]
+(define_memory_constraint "ZB"
+  "@internal
+  An address that is held in a general-purpose register.
+  The offset is zero"
+  (and (match_code "mem")
+   (match_test "GET_CODE (XEXP (op,0)) == REG")))

It'd be good to use things like REG_P in new code.

Formatting nit: should be a space after “op,”.


[…]
+;; Pipeline descriptions.
+;;
+;; generic.md provides a fallback for processors without a specific
+;; pipeline description.  It is derived from the old define_function_unit
+;; version and uses the "alu" and "imuldiv" units declared below.
+;;
+;; Some of the processor-specific files are also derived from old
+;; define_function_unit descriptions and simply override the parts of
+;; generic.md that don't apply.  The other processor-specific files
+;; are self-contained.

I don't think these last two paragraphs apply to the new code.
The MIPS generic.md was converted from a much older pipeline
description format.  The conversion meant the older processor
descriptions weren't self-contained and relied on a mixture
of processor-specific things (in their own file) and generic
things (in this file).

New processor descriptions should be self-contained as far
as possible, in terms of not sharing cpu units with other
processor descriptions.


+(define_automaton "alu,imuldiv")
+
+(define_cpu_unit "alu" "alu")
+(define_cpu_unit "imuldiv" "imuldiv")
+
+;; Ghost instructions produce no real code.
+;; They exist purely to express an effect on dataflow.
+(define_insn_reservation "ghost" 0
+  (eq_attr "type" "ghost")
+  "nothing")
+
+;; This file is derived from the old define_function_unit description.
+;; Each reservation can be overridden on a processor-by-processor basis.

Same for this last comment.

The "ghost" reservation is inherently sharable because it doesn't
use any CPU units.  But for a new port, I think the other reservations
in this file should be conditional on a particular -mtune.


[…]
diff --git a/gcc/config/loongarch/la464.md b/gcc/config/loongarch/la464.md
new file mode 100644
index 000..ae3808b51bb
--- /dev/null
+++ b/gcc/config/loongarch/la464.md
@@ -0,0 +1,132 @@
+;; Pipeline model for LoongArch LA464 cores.
+
+;; Copyright (C) 2021-2022 Free Software Foundation, Inc.
+;; Contributed by Loongson Ltd.
+
+;; This file is part of GCC.
+;;
+;; GCC is free software; you can redistribute it and/or modify it
+;; under the terms of the GNU General Public License as published
+;; by the Free Software Foundation; either version 3, or (at your
+;; option) any later version.
+;;
+;; GCC is distributed in the hope that it will be useful, but WITHOUT
+;; ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+;; or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
+;; License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with GCC; see the file COPYING3.  If not see
+;; .
+
+;; Uncomment the following line to output automata for debugging.
+;; (automata_option "v")
+
+;; Automaton for integer instructions.
+(define_automaton "la464_a_alu")
+
+;; Automaton for floating-point instructions.
+(define_automaton "la464_a_falu")
+
+;; Automaton for memory operations.
+(define_automaton "la464_a_mem")
+
+;; Describe the resources.
+
+(define_cpu_unit "la464_alu1" "la464_a_alu")
+(define_cpu_unit "la464_alu2" "la464_a_alu")
+(define_cpu_unit "la464_mem1" "la464_a_mem")
+(define_cpu_unit "la464_mem2" "la464_a_mem")
+(define_cpu_unit "la464_falu1" "la464_a_falu")
+(define_cpu_unit "la464_falu2" "la464_a_falu")
+
+;; Describe instruction reservations.
+
+(define_insn_reservation "la464_arith" 1
+  (and (match_test "TARGET_ARCH_LA464")

Normally scheduling should be determined by -mtune (with a default
-mtune chosen by -march when no explicit -mtune is given).  So I was
surprised to see this testing TARGET_ARCH_* instead of TARGET_TUNE_*.


+   (eq_attr "type" "arith,clz,co

Re: [PATCH v8 04/12] LoongArch Port: Machine description files.

[程璐璐]

Hi,

   We are modifying the code, this support will be

added in the next commit.

Thanks.

在 2022/3/8 上午4:15, Xi Ruoyao 写道:

On Fri, 2022-03-04 at 15:18 +0800, xucheng...@loongson.cn wrote:


 * config/loongarch/loongarch.md: New file.

An ICE happens building OpenSSH-8.9p1.  Investigation shows it's caused
by the flag "-fzero-call-used-regs=".  It's because the compiler
attempts to clear FCCx registers but can't figure out how.

This flag also triggers ICE for other targets (for example, PR 104820
for MIPS), and the related tests (zero-scratch-regs-{8,9,10,11}.c) are
marked dg-skip for many targets.

But it's unfortunate that packages like OpenSSH have already start to
use this flag... I guess they just enabled it once they saw it was
working for i386 :(.  So it's better to solve the problem for a new
target.

A "quick fix" is adding an insn to clear FCCx.  This is enough to build
OpenSSH and make zero-scratch-regs-{8,9,10,11}.c PASS.

diff --git a/gcc/config/loongarch/loongarch.md 
b/gcc/config/loongarch/loongarch.md
index a9a8bc4b038..76c5ded9fe4 100644
--- a/gcc/config/loongarch/loongarch.md
+++ b/gcc/config/loongarch/loongarch.md
@@ -2020,6 +2020,12 @@
DONE;
  })
  
+;; Clear one FCC register

+(define_insn "movfcc" [(set (match_operand:FCC 0 "register_operand" "=z")
+ (const_int 0))]
+  ""
+  "movgr2cf\t%0,$r0")
+
  ;; Conditional move instructions.
  
  (define_insn "*sel_using_"

Re: [PATCH v8 00/12] Add LoongArch support.

[程璐璐]

在 2022/3/15 下午10:35, Xi Ruoyao 写道:

On Fri, 2022-03-04 at 15:17 +0800, xucheng...@loongson.cn wrote:


v7 -> v8
1. Add new addressing type ADDRESS_REG_REG support.
2. Modify documentation.
3. Eliminate compile-time warnings.

Hi,

The v8 series does not build LoongArch Linux kernel tree
(https://github.com/loongson/linux, loongarch-next branch) successfully.
This is a regression: the v7 series built the kernel fine.

A testcase reduced from the __get_data_asm macro in uaccess.h:

$ cat t1.c
char *ptr;
int offset;
struct m
{
   char a[2];
};

char
x (void)
{
   char t;
   asm volatile("ld.b %0, %1" : "=r"(t) : "o"(*(struct m *)(ptr + offset)));
   return t;
}

$ ./gcc/cc1 t1.c -nostdinc -O
t1.c: In function ‘x’:
t1.c:12:3: error: impossible constraint in ‘asm’
12 |   asm volatile("ld.b %0, %1" : "=r"(t) : "o"(*(struct m *)(ptr + 
offset)));
   |   ^~~

It seems changing the constraint "o" to "m" can work around this issue.
I'm not sure if this is a compiler bug or a kernel bug.


Hi,

LoongArch supports memory modes as follows:

 mode        memory constraint

1. base  + index "k"

2. base + imm12 "m"

3. base + imm16 (immediate 4-byte alignment) "ZC"

4. base + 0 "ZB"

so, this constraint should be "m".

Re: [PATCH v8 05/12] LoongArch Port: Machine description C files and .h files.

[程璐璐]

在 2022/3/8 上午2:17, Richard Sandiford 写道:

[…]
+/* This definition replaces the formerly used 'm' constraint with a
+   different constraint letter in order to avoid changing semantics of
+   the 'm' constraint when accepting new address formats in
+   TARGET_LEGITIMATE_ADDRESS_P.  The constraint letter defined here
+   must not be used in insn definitions or inline assemblies.  */
+#define TARGET_MEM_CONSTRAINT 'w'

Do you need to do this for a new port like Loongson?  It looks like
TARGET_LEGITIMATE_ADDRESS_P accepts all valid forms of address,
including reg+reg (good!), so shouldn't "m" accept them too?

If there is already existing code that assumes that "m" is never indexed
then the definition obviously makes sense.  But if you don't know of any
such code then it would be better to make "m" accept all the things that
TARGET_LEGITIMATE_ADDRESS_P does (by removing this definition).


Hi, Richard:

LoongArch supports memory modes as follows:

 mode

1. base  + index

2. base + imm12

3. base + imm16 (immediate 4-byte alignment)

4. base + 0

Because 'base + imm12' is most case, so we defined it to 'm'.


Thanks.

Re: [PATCH v8 04/12] LoongArch Port: Machine description files.

[程璐璐]

在 2022/3/7 上午12:16, Richard Sandiford 写道:

+(define_split
+  [(match_operand 0 "small_data_pattern")]
+  "reload_completed"
+  [(match_dup 0)]
+  { operands[0] = loongarch_rewrite_small_data (operands[0]); })
+
+
+;; Match paired HI/SI/SF/DFmode load/stores.
+(define_insn "*join2_load_store"
+  [(set (match_operand:JOIN_MODE 0 "nonimmediate_operand"
+  "=r,f,m,m,r,ZC,r,k,f,k")
+   (match_operand:JOIN_MODE 1 "nonimmediate_operand" 
"m,m,r,f,ZC,r,k,r,k,f"))
+   (set (match_operand:JOIN_MODE 2 "nonimmediate_operand"
+   "=r,f,m,m,r,ZC,r,k,f,k")
+   (match_operand:JOIN_MODE 3 "nonimmediate_operand" 
"m,m,r,f,ZC,r,k,r,k,f"))]
+  "reload_completed"
+  {
+bool load_p = (which_alternative == 0 || which_alternative == 1);
+/* Reg-renaming pass reuses base register if it is dead after bonded loads.
+   Hardware does not bond those loads, even when they are consecutive.
+   However, order of the loads need to be checked for correctness.  */
+if (!load_p || !reg_overlap_mentioned_p (operands[0], operands[1]))
+  {

I'm not sure I understand how these patterns work, but it looks like the
condition above is trying to work around a later change to the insn by
regrename, after peephole2 has checked loongarch_load_store_bonding_p.
If so, you should be able to avoid that by marking the destinations of
the loads as earlyclobbers, using "&r" instead of "r" for the first
alternative.  regrename should then preserve the conditions that
loongarch_load_store_bonding_p checked earlier.

Same for the other patterns.


Hi,

I think peephole pass is after reload pass, so peephole pass don't need '&'.


Thanks.

Re: [PATCH v10 10/12] LoongArch Port: libgomp

[程璐璐]

Hi, Jakub:

 Thanks for your review.

在 2022/3/24 下午8:41, Jakub Jelinek 写道:

On Thu, Mar 24, 2022 at 08:37:32PM +0800, chenglulu wrote:

libgomp/

 * configure.tgt: Add LoongArch triplet.

Ok.


diff --git a/libgomp/configure.tgt b/libgomp/configure.tgt
index d4f1e741b5a..2cd7272fcd8 100644
--- a/libgomp/configure.tgt
+++ b/libgomp/configure.tgt
@@ -56,6 +56,10 @@ if test x$enable_linux_futex = xyes; then
config_path="linux/ia64 linux posix"
;;
  
+loongarch*-*-linux*)

+   config_path="linux posix"
+   ;;
+
  mips*-*-linux*)
config_path="linux/mips linux posix"
;;
--
2.31.1

Jakub

Re: [PATCH] loongarch: testsuite: skip builtin-apply2.c

[程璐璐]

在 2022/4/9 上午5:46, Xi Ruoyao 写道:

A simple testcase change, tested on loongarch64-linux-gnuabif64.  Ok for trunk?

---

On LoongArch, variadic functions use different arugment passing
conventions so this test is not valid (see the section named "Variadic
argument" in the [ELF ABI][1]) and should be skipped.

[1]: 
https://loongson.github.io/LoongArch-Documentation/LoongArch-ELF-ABI-EN.html

gcc/testsuite/

* gcc.dg/builtin-apply2.c (dg-skip-if): Add loongarch*-*-*.
---
  gcc/testsuite/gcc.dg/builtin-apply2.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/builtin-apply2.c 
b/gcc/testsuite/gcc.dg/builtin-apply2.c
index 9049af5821f..0f350f4ac16 100644
--- a/gcc/testsuite/gcc.dg/builtin-apply2.c
+++ b/gcc/testsuite/gcc.dg/builtin-apply2.c
@@ -1,7 +1,7 @@
  /* { dg-do run } */
  /* { dg-require-effective-target untyped_assembly } */
  /* { dg-skip-if "Variadic funcs have all args on stack. Normal funcs have args in 
registers." { "avr-*-* nds32*-*-* amdgcn-*-*" } } */
-/* { dg-skip-if "Variadic funcs use different argument passing from normal funcs." { 
"csky*-*-* riscv*-*-* or1k*-*-* msp430-*-* pru-*-*" } } */
+/* { dg-skip-if "Variadic funcs use different argument passing from normal funcs." { 
"csky*-*-* riscv*-*-* or1k*-*-* msp430-*-* pru-*-* loongarch*-*-*" } } */
  /* { dg-skip-if "Variadic funcs use Base AAPCS.  Normal funcs use VFP variant." { 
arm*-*-* && arm_hf_eabi } } */
  
  /* PR target/12503 */



OK.

Thanks!

LuluCheng

Re: [PATCH v5 02/12] LoongArch Port: gcc build

[程璐璐]

在 2022/1/22 下午4:46, Xi Ruoyao 写道:

On Sat, 2022-01-22 at 09:41 +0100, Jakub Jelinek wrote:

On Sat, Jan 22, 2022 at 04:33:08PM +0800, Xi Ruoyao via Gcc-patches
wrote:

On Sat, 2022-01-22 at 15:55 +0800, Chenghua Xu wrote:

+$(srcdir)/config/loongarch/loongarch.opt: \
+   $(srcdir)/config/loongarch/genopts/genstr.sh \
+   $(srcdir)/config/loongarch/genopts/loongarch.opt.in
+   $(SHELL) $< opt > $@
+
+$(LA_STR_H): \
+   $(srcdir)/config/loongarch/genopts/genstr.sh \
+   $(srcdir)/config/loongarch/genopts/loongarch-strings
+   $(SHELL) $< header > $@

I'm not sure about these two rules: is it allowed to modify files in
$(srcdir) running "make"?

No (except in --enable-maintainer-mode), but the above certainly
looks wrong, both should be in the objdir instead.

Does the building system support a .opt file in $(objdir)?  And how to
handle the translations (.po) for loongarch.opt if it's in $(objdir)?


Under the MIPS architecture, *.opt files are also generated in $(srcdir).

Re: [PATCH v5 02/12] LoongArch Port: gcc build

[程璐璐]

在 2022/1/22 下午5:05, Xi Ruoyao 写道:

On Sat, 2022-01-22 at 16:56 +0800, 程璐璐 wrote:


Under the MIPS architecture, *.opt files are also generated in
$(srcdir).

Well, but then you should put the commands for generating those files
into contrib/gcc_update instead of Makefile.in, just like MIPS.

Sorry, I will move this commands to contrib/gcc_update.

Re: Re: [PATCH v5 02/12] LoongArch Port: gcc build

[程璐璐]

> -原始邮件-
> 发件人: "Jakub Jelinek" 
> 发送时间: 2022-01-22 17:31:40 (星期六)
> 收件人: "Xi Ruoyao" 
> 抄送: "程璐璐" , "Chenghua Xu" , 
gcc-patches@gcc.gnu.org, jos...@codesourcery.com
> 主题: Re: [PATCH v5 02/12] LoongArch Port: gcc build
> 
> On Sat, Jan 22, 2022 at 05:05:00PM +0800, Xi Ruoyao wrote:
> > On Sat, 2022-01-22 at 16:56 +0800, 程璐璐 wrote:
> > 
> > > Under the MIPS architecture, *.opt files are also generated in
> > > $(srcdir).
> > 
> > Well, but then you should put the commands for generating those files
> > into contrib/gcc_update instead of Makefile.in, just like MIPS.
> 
> The MIPS does it wrong too.  If the generated file is checked into the
> tree as in the mips case, such a make rule can be there, but it
> should be wrapped with
> ifeq($(ENABLE_MAINTAINER_RULES),true)
> ...
> endif
> or so.
> The point is, when not --enable-maintainer-mode, the gcc tree can be stored
> on read-only filesystem.
> When adding new CPUs, one just uses --enable-maintainer-mode and regenates
> the generated but committed files and commits them together with the
> changes.
> There are many other examples of such generated files, configure,
> Makefile.in from Makefile.am, etc.
> 
>Jakub

Ok,thanks! We will modify as soon as possible!

--
程璐璐

芯片研发部 

-
龙芯中科技术有限公司
Loongson Technology Corporation Limited
北京市海淀区北清路中关村环保科技示范园 龙芯产业园 2号楼
Loongson Industrial Park, building 2 , Zhongguancun Environmental 
protection park, Haidian District, Beijing, China
邮编: 100095
电话(Tel):  +86 (10) 62546668-5049
邮箱(Email):  chengl...@loongson.cn
--


北京市海淀区中关村环保科技示范园龙芯产业园2号楼100095
电话: +86 (10) 62546668
传真: +86 (10) 62600826
www.loongson.cn


本邮件及其附件含有龙芯中科的商业秘密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制或散发）本邮件及其附件中的信息。如果您错收本邮件，请您立即电话或邮件通知发件人并删除本邮件。
 
This email and its attachments contain confidential information from Loongson 
Technology , which is intended only for the person or entity whose address is 
listed above. Any use of the information contained herein in any way 
(including, but not limited to, total or partial disclosure, reproduction or 
dissemination) by persons other than the intended recipient(s) is prohibited. 
If you receive this email in error, please notify the sender by phone or email 
immediately and delete it. 

Re: [PATCH v5 02/12] LoongArch Port: gcc build

[程璐璐]

在 2022/1/22 下午5:31, Jakub Jelinek 写道:

On Sat, Jan 22, 2022 at 05:05:00PM +0800, Xi Ruoyao wrote:

On Sat, 2022-01-22 at 16:56 +0800, 程璐璐 wrote:


Under the MIPS architecture, *.opt files are also generated in
$(srcdir).

Well, but then you should put the commands for generating those files
into contrib/gcc_update instead of Makefile.in, just like MIPS.

The MIPS does it wrong too.  If the generated file is checked into the
tree as in the mips case, such a make rule can be there, but it
should be wrapped with
ifeq($(ENABLE_MAINTAINER_RULES),true)
...
endif
or so.
The point is, when not --enable-maintainer-mode, the gcc tree can be stored
on read-only filesystem.
When adding new CPUs, one just uses --enable-maintainer-mode and regenates
the generated but committed files and commits them together with the
changes.
There are many other examples of such generated files, configure,
Makefile.in from Makefile.am, etc.

Jakub


Hi, Mr Jakub:

I have fixed it, could you please take a look.


diff --git a/contrib/gcc_update b/contrib/gcc_update
index 1cf15f9b3c2..641ce164775 100755
--- a/contrib/gcc_update
+++ b/contrib/gcc_update
@@ -86,6 +86,8 @@ gcc/config/arm/arm-tables.opt: 
gcc/config/arm/arm-cpus.in gcc/config/arm/parsecp
 gcc/config/c6x/c6x-tables.opt: gcc/config/c6x/c6x-isas.def 
gcc/config/c6x/genopt.sh
 gcc/config/c6x/c6x-sched.md: gcc/config/c6x/c6x-sched.md.in 
gcc/config/c6x/gensched.sh
 gcc/config/c6x/c6x-mult.md: gcc/config/c6x/c6x-mult.md.in 
gcc/config/c6x/genmult.sh
+gcc/config/loongarch/loongarch-str.h: 
gcc/config/loongarch/genopts/genstr.sh 
gcc/config/loongarch/genopts/loongarch-string
+gcc/config/loongarch/loongarch.opt: 
gcc/config/loongarch/genopts/genstr.sh 
gcc/config/loongarch/genopts/loongarch.opt.in
 gcc/config/m68k/m68k-tables.opt: gcc/config/m68k/m68k-devices.def 
gcc/config/m68k/m68k-isas.def gcc/config/m68k/m68k-microarchs.def 
gcc/config/m68k/genopt.sh
 gcc/config/mips/mips-tables.opt: gcc/config/mips/mips-cpus.def 
gcc/config/mips/genopt.sh
 gcc/config/rs6000/rs6000-tables.opt: gcc/config/rs6000/rs6000-cpus.def 
gcc/config/rs6000/genopt.sh


diff --git a/gcc/config/loongarch/t-loongarch 
b/gcc/config/loongarch/t-loongarch

index 6ed1a3ab56a..c106be1ec45 100644
--- a/gcc/config/loongarch/t-loongarch
+++ b/gcc/config/loongarch/t-loongarch
@@ -21,7 +21,15 @@ LA_MULTIARCH_TRIPLET = $(patsubst 
LA_MULTIARCH_TRIPLET=%,%,$\

 $(filter LA_MULTIARCH_TRIPLET=%,$(tm_defines)))

 # String definition header
-LA_STR_H = $(srcdir)/config/loongarch/loongarch-str.h
+$(srcdir)/config/loongarch/loongarch-str.h: s-loongarch-str ; @true
+s-loongarch-str: $(srcdir)/config/loongarch/genopts/genstr.sh \
+   $(srcdir)/config/loongarch/genopts/loongarch-strings
+   $(SHELL) $(srcdir)/config/loongarch/genopts/genstr.sh header \
+    $(srcdir)/config/loongarch/genopts/loongarch-strings > \
+    tmp-loongarch-str.h
+   $(SHELL) $(srcdir)/../move-if-change tmp-loongarch-str.h \
+   $(srcdir)/config/loongarch/loongarch-str.h
+   $(STAMP) s-loongarch-str

 loongarch-c.o: $(srcdir)/config/loongarch/loongarch-c.cc $(CONFIG_H) 
$(SYSTEM_H) \

    coretypes.h $(TM_H) $(TREE_H) output.h $(C_COMMON_H) $(TARGET_H)
@@ -48,12 +56,13 @@ loongarch-cpu.o: 
$(srcdir)/config/loongarch/loongarch-cpu.cc $(LA_STR_H)

 loongarch-def.o: $(srcdir)/config/loongarch/loongarch-def.c $(LA_STR_H)
    $(CC) -c $(ALL_CFLAGS) $(INCLUDES) $<

-$(srcdir)/config/loongarch/loongarch.opt: \
-   $(srcdir)/config/loongarch/genopts/genstr.sh \
+$(srcdir)/config/loongarch/loongarch.opt: s-loongarch-opt ; @true
+s-loongarch-opt: $(srcdir)/config/loongarch/genopts/genstr.sh \
    $(srcdir)/config/loongarch/genopts/loongarch.opt.in
-   $(SHELL) $< opt > $@
+   $(SHELL) $(srcdir)/config/loongarch/genopts/genstr.sh opt \
+    $(srcdir)/config/loongarch/genopts/loongarch.opt.in \
+    > tmp-loongarch.opt
+   $(SHELL) $(srcdir)/../move-if-change tmp-loongarch.opt \
+    $(srcdir)/config/loongarch/loongarch.opt
+   $(STAMP) s-loongarch-opt

-$(LA_STR_H): \
-   $(srcdir)/config/loongarch/genopts/genstr.sh \
-   $(srcdir)/config/loongarch/genopts/loongarch-strings
-   $(SHELL) $< header > $@

Re: [PATCH v5 02/12] LoongArch Port: gcc build

[程璐璐]

在 2022/1/22 下午4:42, Xi Ruoyao 写道:

On Sat, 2022-01-22 at 15:55 +0800, Chenghua Xu wrote:

+mstrict-align
+Target Var(TARGET_STRICT_ALIGN) Init(0)
+Do not generate unaligned memory accesses.

Section 2.1.8 of LoongArch spec says "load/store instruction *may* be
implemented to allow unaligned memory access".  As it's not a "must",
should we really enable this by default?


Currently, our main architecture supports this feature. Later, in the 
compiler the value of this macro will be set according to the architecture.

Re: [PATCH v5 02/12] LoongArch Port: gcc build

[程璐璐]

在 2022/1/23 下午5:00, Xi Ruoyao 写道:

On Sun, 2022-01-23 at 16:39 +0800, 程璐璐 wrote:

在 2022/1/22 下午4:42, Xi Ruoyao 写道:
  


On Sat, 2022-01-22 at 15:55 +0800, Chenghua Xu wrote:
  


+mstrict-align
+Target Var(TARGET_STRICT_ALIGN) Init(0)
+Do not generate unaligned memory accesses.

Section 2.1.8 of LoongArch spec says "load/store instruction *may* be
implemented to allow unaligned memory access".  As it's not a "must",
should we really enable this by default?

Currently, our main architecture supports this feature. Later, in the compiler
the value of this macro will be set according to the architecture.

My suggestion is to make -mstrict-align default for the default
('-march=loongarch64'), as it should generate code working on every
architecture following the spec.  And, -mno-strict-align should be the
default of -march=la464, which is the current main architecture
supporting unaligned access.


Ok we will consider your suggestion!

Re: [PATCH v7 11/12] LoongArch Port: gcc/testsuite

[程璐璐]

Thanks,  speculation barrier is not needed for loongarch.

I have removed the warning.

在 2022/2/25 上午3:32, Xi Ruoyao 写道:

On Sat, 2022-02-12 at 11:11 +0800, xucheng...@loongson.cn wrote:

From: chenglulu 

2022-02-12  Chenghua Xu  
     Lulu Cheng  

gcc/testsuite/

spec-barrier tests fail with:

./testsuite/c-c++-common/spec-barrier-1.c:21:3: warning: this target
does not define a speculation barrier; your program will still execute
correctly, but incorrect speculation may not be restricted

I'd seen some news saying your uarch has in-silicon defense for
speculation related vulnerabilities.  If this is true you can just make
__builtin_speculation_safe_value a nop.  Quote from gcc internal doc:


  If this pattern is not defined then the default expansion of
  '__builtin_speculation_safe_value' will emit a warning.  You can
  suppress this warning by defining this pattern with a final
  condition of '0' (zero), which tells the compiler that a
  speculation barrier is not needed for this target.

Re: [PATCH v4 04/12] LoongArch Port: Machine Decsription files.

[程璐璐]

在 2022/1/7 上午1:54, Xi Ruoyao 写道:

On Fri, 2021-12-24 at 17:28 +0800, chenglulu wrote:

+(define_insn "*zero_extendsidi2_internal"
+  [(set (match_operand:DI 0 "register_operand" "=r,r,r")
+   (subreg:DI (match_operand:SI 1 "nonimmediate_operand" "r,ZC,W") 0))]
+  "TARGET_64BIT"
+  "@
+   bstrpick.d\t%0,%1,31,0
+   ldptr.w\t%0,%1\n\tlu32i.d\t%0,0
+   ld.wu\t%0,%1"
+  [(set_attr "move_type" "arith,load,load")
+   (set_attr "mode" "DI")
+   (set_attr "insn_count" "1,2,1")])

This pattern is generating wrong code, causing

FAIL: gcc.dg/compat/scalar-by-value-3 c_compat_x_tst.o-c_compat_y_tst.o execute

This failure is a real bug, the reduced testcase is attached.  In the
assembly:

 # ...
 bstrins.d   $r5,$r13,31,0
 addi.d  $r12,$r22,-228
 bstrpick.d  $r12,$r12,31,0
 bstrins.d   $r5,$r12,63,32
 addi.w  $r4,$r0,14  # 0xe
 bl  testvaci

This obviously does not make any sense: it calculates the *address* of
g[0]'s imaginary part, truncates it to 32-bit, then pass it as the
*value* of the imaginary part to testvaci().

The problem is: before the reload pass, the compiler consider g[0] a
(virtual) register.  It only becomes MEM after the reload pass.  Adding
reload_completed as the condition of this insn seems able to fix the
issue.  However I'm not sure if other insns need the change too.


+;; See the comment before the *and3 pattern why this is generated by
+;; combine.

A minor issue: the comment before 'and3' does not exist.


Thanks, we are working on this. In addition, we will check other 
instruction templates.