[PATCH, V1 0/1] RISC-V: Make R_RISCV_SUB6 conforms to riscv abi standard

[zengxiao]

From: zengxiao 

Hi all RISC-V folks:

When riscv-objdump is used to generate dwarf information, problems are found, 
like:
DW_CFA_??? (User defined call frame op: 0x3c)

This error is related to that riscv-objdump does not follow the riscv 
R_RISCV_SUB6 standard. 
Riscv-readelf is correct because it follows the R_RISCV_SUB6 standard.

There are test cases in 
https://github.com/zeng-xiao/gnu-bug-fix/tree/main/EG-769
that describe the error in detail. 

zengxiao (1):
  RISC-V: Make R_RISCV_SUB6 conforms to riscv abi standard

 bfd/elfxx-riscv.c |  7 +
 .../testsuite/binutils-all/riscv/dwarf-SUB6.d | 31 +++
 .../testsuite/binutils-all/riscv/dwarf-SUB6.s | 12 +++
 3 files changed, 50 insertions(+)
 create mode 100644 binutils/testsuite/binutils-all/riscv/dwarf-SUB6.d
 create mode 100644 binutils/testsuite/binutils-all/riscv/dwarf-SUB6.s

-- 
2.34.1

[PATCH, V1 1/1] RISC-V: Make R_RISCV_SUB6 conforms to riscv abi standard

[zengxiao]

From: zengxiao 

This patch makes R_RISCV_SUB6 conforms to riscv abi standard.
R_RISCV_SUB6 only the lower 6 bits of the code are valid.
The proposed specification which can be found in 8.5. Relocations of,
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/releases/download/v1.0-rc4/riscv-abi.pdf

bfd/ChangeLog:

* elfxx-riscv.c (riscv_elf_add_sub_reloc):

binutils/ChangeLog:

* testsuite/binutils-all/riscv/dwarf-SUB6.d: New test.
* testsuite/binutils-all/riscv/dwarf-SUB6.s: New test.

reviewed-by: gao...@eswincomputing.com
 jinyanji...@eswincomputing.com

Signed-off-by: zengxiao 
---
 bfd/elfxx-riscv.c |  7 +
 .../testsuite/binutils-all/riscv/dwarf-SUB6.d | 31 +++
 .../testsuite/binutils-all/riscv/dwarf-SUB6.s | 12 +++
 3 files changed, 50 insertions(+)
 create mode 100644 binutils/testsuite/binutils-all/riscv/dwarf-SUB6.d
 create mode 100644 binutils/testsuite/binutils-all/riscv/dwarf-SUB6.s

diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
index 300ccf49534..e71d4a456f2 100644
--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -994,6 +994,13 @@ riscv_elf_add_sub_reloc (bfd *abfd,
   relocation = old_value + relocation;
   break;
 case R_RISCV_SUB6:
+  {
+bfd_vma six_bit_valid_value = old_value & howto->dst_mask;
+six_bit_valid_value -= relocation;
+relocation = (six_bit_valid_value & howto->dst_mask) |
+ (old_value & ~howto->dst_mask);
+  }
+  break;
 case R_RISCV_SUB8:
 case R_RISCV_SUB16:
 case R_RISCV_SUB32:
diff --git a/binutils/testsuite/binutils-all/riscv/dwarf-SUB6.d 
b/binutils/testsuite/binutils-all/riscv/dwarf-SUB6.d
new file mode 100644
index 000..47d5ae570d7
--- /dev/null
+++ b/binutils/testsuite/binutils-all/riscv/dwarf-SUB6.d
@@ -0,0 +1,31 @@
+#PROG: objcopy
+#objdump: --dwarf=frames
+
+tmpdir/riscvcopy.o: file format elf32-littleriscv
+
+Contents of the .eh_frame section:
+
+
+ 0020  CIE
+  Version:   3
+  Augmentation:  "zR"
+  Code alignment factor: 1
+  Data alignment factor: -4
+  Return address column: 1
+  Augmentation data: 1b
+  DW_CFA_def_cfa_register: r2 \(sp\)
+  DW_CFA_def_cfa_offset: 48
+  DW_CFA_offset: r1 \(ra\) at cfa-4
+  DW_CFA_offset: r8 \(s0\) at cfa-8
+  DW_CFA_def_cfa: r8 \(s0\) ofs 0
+  DW_CFA_restore: r1 \(ra\)
+  DW_CFA_restore: r8 \(s0\)
+  DW_CFA_def_cfa: r2 \(sp\) ofs 48
+  DW_CFA_def_cfa_offset: 0
+  DW_CFA_nop
+
+0024 0010 0028 FDE cie= pc=002c..002c
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
diff --git a/binutils/testsuite/binutils-all/riscv/dwarf-SUB6.s 
b/binutils/testsuite/binutils-all/riscv/dwarf-SUB6.s
new file mode 100644
index 000..fe959f59d9b
--- /dev/null
+++ b/binutils/testsuite/binutils-all/riscv/dwarf-SUB6.s
@@ -0,0 +1,12 @@
+.attribute arch, "rv32i2p0_m2p0_a2p0_f2p0_c2p0"
+.cfi_startproc
+.cfi_def_cfa_offset 48
+.cfi_offset 1, -4
+.cfi_offset 8, -8
+.cfi_def_cfa 8, 0
+.cfi_restore 1
+.cfi_restore 8
+.cfi_def_cfa 2, 48
+.cfi_def_cfa_offset 0
+.cfi_endproc
+
\ No newline at end of file
-- 
2.34.1

[PATCH v2 2/2] RISC-V: Optimize RVV epilogue logic.

[jiawei]

Sometimes "step1 -= scalable_frame" will cause adjust equal to
zero. And it will generate additional redundant instruction 
"addi sp,sp,0". Add checking segement to skip that case.

gcc/ChangeLog:

* config/riscv/riscv.cc (riscv_expand_epilogue): 
New check segement.

---
 gcc/config/riscv/riscv.cc | 35 +++
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 02a01ca0b7c..433b9b13eb6 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -5185,25 +5185,28 @@ riscv_expand_epilogue (int style)
  step1 -= scalable_frame;
}
 
-  /* Get an rtx for STEP1 that we can add to BASE.  */
-  rtx adjust = GEN_INT (step1.to_constant ());
-  if (!SMALL_OPERAND (step1.to_constant ()))
-   {
- riscv_emit_move (RISCV_PROLOGUE_TEMP (Pmode), adjust);
- adjust = RISCV_PROLOGUE_TEMP (Pmode);
-   }
-
-  insn = emit_insn (
-  gen_add3_insn (stack_pointer_rtx, stack_pointer_rtx, adjust));
-
-  rtx dwarf = NULL_RTX;
-  rtx cfa_adjust_rtx = gen_rtx_PLUS (Pmode, stack_pointer_rtx,
+  /* Get an rtx for STEP1 that we can add to BASE.  
+ Skip if adjust equal to zero.  */
+  if (step1.to_constant () != 0)
+  {
+rtx adjust = GEN_INT (step1.to_constant ());
+if (!SMALL_OPERAND (step1.to_constant ()))
+{
+  riscv_emit_move (RISCV_PROLOGUE_TEMP (Pmode), adjust);
+  adjust = RISCV_PROLOGUE_TEMP (Pmode);
+}
+
+insn = emit_insn (
+   gen_add3_insn (stack_pointer_rtx, stack_pointer_rtx, adjust));
+rtx dwarf = NULL_RTX;
+rtx cfa_adjust_rtx = gen_rtx_PLUS (Pmode, stack_pointer_rtx,
 GEN_INT (step2));
 
-  dwarf = alloc_reg_note (REG_CFA_DEF_CFA, cfa_adjust_rtx, dwarf);
-  RTX_FRAME_RELATED_P (insn) = 1;
+dwarf = alloc_reg_note (REG_CFA_DEF_CFA, cfa_adjust_rtx, dwarf);
+RTX_FRAME_RELATED_P (insn) = 1;
 
-  REG_NOTES (insn) = dwarf;
+REG_NOTES (insn) = dwarf;
+ }
 }
   else if (frame_pointer_needed)
 {
-- 
2.25.1

[PATCH v2 0/2] RISC-V: Optimize RVV epilogue logic.

[jiawei]

Current epilogue will generate "addi sp,sp,0" redundant instruction.

```
csrrt0,vlenb
sllit1,t0,1
add sp,sp,t1
addisp,sp,0
ld  s0,24(sp)
addisp,sp,32
jr  ra
```

Optimize it by check if adjust equal to zero, remove redundant insn gen.

```
csrrt0,vlenb
sllit1,t0,1
add sp,sp,t1
ld  s0,24(sp)
addisp,sp,32
jr  ra
```

Thanks for Kito and Jeff's suggestion, add testcase and fix code format.

jiawei (2):
  RISC-V: Add spill sp adjust check testcase.
  RISC-V: Optimize RVV epilogue logic.

 gcc/config/riscv/riscv.cc | 35 ++-
 .../riscv/rvv/base/spill-sp-adjust.c  | 13 +++
 2 files changed, 32 insertions(+), 16 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/spill-sp-adjust.c

-- 
2.25.1

[PATCH v2 1/2] RISC-V: Add spill sp adjust check testcase.

[jiawei]

This testcase mix exist spill-1.c and adding new fun to check if
there have redundant addi intructions. Idea provided by Jeff Law.

gcc/testsuite/ChangeLog:

* gcc.target/riscv/rvv/base/spill-sp-adjust.c: New test.

---
 .../gcc.target/riscv/rvv/base/spill-sp-adjust.c | 13 +
 1 file changed, 13 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/spill-sp-adjust.c

diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/spill-sp-adjust.c 
b/gcc/testsuite/gcc.target/riscv/rvv/base/spill-sp-adjust.c
new file mode 100644
index 000..0226554abf3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/spill-sp-adjust.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv" } */
+
+#include "spill-1.c"
+
+void
+spill_sp_adjust (int8_t *v)
+{
+  vint8mf8_t v1 = *(vint8mf8_t*)v; 
+}
+
+/* Make sure we do not have a useless SP adjustment.  */
+/* { dg-final { scan-assembler-not "addi\tsp,sp,0" } } */
-- 
2.25.1

Re: [PATCH 3/8]middle-end: Support extractions of subvectors from arbitrary element position inside a vector

[Hongtao Liu via Gcc-patches]

Hi:
  I'm from https://gcc.gnu.org/pipermail/gcc-patches/2022-November/606040.html.
>  }
>
>/* See if we can get a better vector mode before extracting.  */
> diff --git a/gcc/optabs.cc b/gcc/optabs.cc
> index 
> cff37ccb0dfc3dd79b97d0abfd872f340855dc96..f338df410265dfe55b6896160090a453cc6a28d9
>  100644
> --- a/gcc/optabs.cc
> +++ b/gcc/optabs.cc
> @@ -6267,6 +6267,7 @@ expand_vec_perm_const (machine_mode mode, rtx v0, rtx 
> v1,
>v0_qi = gen_lowpart (qimode, v0);
>v1_qi = gen_lowpart (qimode, v1);
>if (targetm.vectorize.vec_perm_const != NULL
> + && targetm.can_change_mode_class (mode, qimode, ALL_REGS)
It looks like you want to guard gen_lowpart, shouldn't it be better to
use validate_subreg  or (tmp = gen_lowpart_if_possible (mode,
target_qi)).
IMHO, targetm.can_change_mode_class is mostly used for RA, but not to
guard gen_lowpart.
I did similar things in
https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579296.html
(and ALL_REGS doesn't cover all cases for registers which are both
available for qimode and mode, ALL_REGS fail doesn't mean it can't be
subreg, it just means parts of ALL_REGS can't be subreg. but with a
subset of ALL_REGS, there could be a reg class which return true for
targetm.can_change_mode_class)
>   && targetm.vectorize.vec_perm_const (qimode, qimode, target_qi, 
> v0_qi,
>v1_qi, qimode_indices))
> return gen_lowpart (mode, target_qi);
> @@ -6311,7 +6312,8 @@ expand_vec_perm_const (machine_mode mode, rtx v0, rtx 
> v1,
>  }
>
>if (qimode != VOIDmode
> -  && selector_fits_mode_p (qimode, qimode_indices))
> +  && selector_fits_mode_p (qimode, qimode_indices)
> +  && targetm.can_change_mode_class (mode, qimode, ALL_REGS))
>  {
>icode = direct_optab_handler (vec_perm_optab, qimode);
>if (icode != CODE_FOR_nothing)
> diff --git a/gcc/testsuite/gcc.target/aarch64/ext_1.c 
> b/gcc/testsuite/gcc.target/aarch64/ext_1.c
> new file mode 100644
> index 
> ..18a10a14f1161584267a8472e571b3bc2ddf887a




-- 
BR,
Hongtao

Re: [PATCH v2 0/2] RISC-V: Optimize RVV epilogue logic.

[juzhe.zh...@rivai.ai]

LGTM. Thanks for fixing my mistake. 
Let's see whether other RISC-V folks are happy with this patch.



juzhe.zh...@rivai.ai
 
From: jiawei
Date: 2022-11-15 16:33
To: gcc-patches
CC: kito.cheng; palmer; juzhe.zhong; jeffreyalaw; christoph.muellner; 
philipp.tomsich; wuwei2016; jiawei
Subject: [PATCH v2 0/2] RISC-V: Optimize RVV epilogue logic.
Current epilogue will generate "addi sp,sp,0" redundant instruction.
 
```
csrrt0,vlenb
sllit1,t0,1
add sp,sp,t1
addisp,sp,0
ld  s0,24(sp)
addisp,sp,32
jr  ra
```
 
Optimize it by check if adjust equal to zero, remove redundant insn gen.
 
```
csrrt0,vlenb
sllit1,t0,1
add sp,sp,t1
ld  s0,24(sp)
addisp,sp,32
jr  ra
```
 
Thanks for Kito and Jeff's suggestion, add testcase and fix code format.
 
jiawei (2):
  RISC-V: Add spill sp adjust check testcase.
  RISC-V: Optimize RVV epilogue logic.
 
gcc/config/riscv/riscv.cc | 35 ++-
.../riscv/rvv/base/spill-sp-adjust.c  | 13 +++
2 files changed, 32 insertions(+), 16 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/spill-sp-adjust.c
 
-- 
2.25.1
 
 

RE: [PATCH 3/8]middle-end: Support extractions of subvectors from arbitrary element position inside a vector

[Tamar Christina via Gcc-patches]

> -Original Message-
> From: Hongtao Liu 
> Sent: Tuesday, November 15, 2022 8:36 AM
> To: Tamar Christina 
> Cc: Richard Sandiford ; Tamar Christina via
> Gcc-patches ; nd ;
> rguent...@suse.de
> Subject: Re: [PATCH 3/8]middle-end: Support extractions of subvectors from
> arbitrary element position inside a vector
> 
> Hi:
>   I'm from https://gcc.gnu.org/pipermail/gcc-patches/2022-
> November/606040.html.
> >  }
> >
> >/* See if we can get a better vector mode before extracting.  */
> > diff --git a/gcc/optabs.cc b/gcc/optabs.cc index
> >
> cff37ccb0dfc3dd79b97d0abfd872f340855dc96..f338df410265dfe55b689616009
> 0
> > a453cc6a28d9 100644
> > --- a/gcc/optabs.cc
> > +++ b/gcc/optabs.cc
> > @@ -6267,6 +6267,7 @@ expand_vec_perm_const (machine_mode mode,
> rtx v0, rtx v1,
> >v0_qi = gen_lowpart (qimode, v0);
> >v1_qi = gen_lowpart (qimode, v1);
> >if (targetm.vectorize.vec_perm_const != NULL
> > + && targetm.can_change_mode_class (mode, qimode, ALL_REGS)
> It looks like you want to guard gen_lowpart, shouldn't it be better to use
> validate_subreg  or (tmp = gen_lowpart_if_possible (mode, target_qi)).
> IMHO, targetm.can_change_mode_class is mostly used for RA, but not to
> guard gen_lowpart.

Hmm I don't think this is quite true, there are existing usages in expr.cc and 
rtanal.cc
That do this and aren't part of RA.  As I mentioned before for instance the
canoncalization of vec_select to subreg in rtlanal for instances uses this.

So there are already existing precedence for this.  And the documentation for
the hook says:

"This hook returns true if it is possible to bitcast values held in registers 
of class rclass from mode from to mode to and if doing so preserves the 
low-order bits that are common to both modes. The result is only meaningful if 
rclass has registers that can hold both from and to. The default implementation 
returns true"

So it looks like it's use outside of RA is perfectly valid.. and the 
documentation also mentions
in the example the use from the mid-end as an example.

But if the mid-end maintainers are happy I'll use something else.

Tamar

> I did similar things in
> https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579296.html
> (and ALL_REGS doesn't cover all cases for registers which are both available
> for qimode and mode, ALL_REGS fail doesn't mean it can't be subreg, it just
> means parts of ALL_REGS can't be subreg. but with a subset of ALL_REGS,
> there could be a reg class which return true for
> targetm.can_change_mode_class)
> >   && targetm.vectorize.vec_perm_const (qimode, qimode, target_qi,
> v0_qi,
> >v1_qi, qimode_indices))
> > return gen_lowpart (mode, target_qi); @@ -6311,7 +6312,8 @@
> > expand_vec_perm_const (machine_mode mode, rtx v0, rtx v1,
> >  }
> >
> >if (qimode != VOIDmode
> > -  && selector_fits_mode_p (qimode, qimode_indices))
> > +  && selector_fits_mode_p (qimode, qimode_indices)
> > +  && targetm.can_change_mode_class (mode, qimode, ALL_REGS))
> >  {
> >icode = direct_optab_handler (vec_perm_optab, qimode);
> >if (icode != CODE_FOR_nothing)
> > diff --git a/gcc/testsuite/gcc.target/aarch64/ext_1.c
> > b/gcc/testsuite/gcc.target/aarch64/ext_1.c
> > new file mode 100644
> > index
> >
> ..18a10a14f1161584267a8472e5
> 71
> > b3bc2ddf887a
> 
> 
> 
> 
> --
> BR,
> Hongtao

[PATCH] c++, v2: Allow attributes on concepts - DR 2428

[Jakub Jelinek via Gcc-patches]

On Mon, Nov 14, 2022 at 07:00:54PM -0500, Jason Merrill wrote:
> > The following patch adds parsing of attributes to concept definition,
> > allows deprecated attribute to be specified (some ugliness needed
> > because CONCEPT_DECL is a cp/*.def attribute and so can't be mentioned
> > in c-family/ directly; used what is used for objc method decls,
> > an alternative would be a langhook)
> 
> Several of the codes in c-common.def are C++-only, you might just move it
> over?
> 
> > and checks TREE_DEPRECATED in
> > build_standard_check (not sure if that is the right spot, or whether
> > it shouldn't be checked also for variable and function concepts and
> > how to write testcase coverage for that).
> 
> I wouldn't bother with var/fn concepts, they're obsolete.

Ok, so like this?
The previous version passed bootstrap/regtest on x86_64-linux and i686-linux,
I'll of course test this one as well.

Jakub

Re: [PATCH] c++, v2: Allow attributes on concepts - DR 2428

[Jakub Jelinek via Gcc-patches]

On Tue, Nov 15, 2022 at 09:54:00AM +0100, Jakub Jelinek via Gcc-patches wrote:
> On Mon, Nov 14, 2022 at 07:00:54PM -0500, Jason Merrill wrote:
> > > The following patch adds parsing of attributes to concept definition,
> > > allows deprecated attribute to be specified (some ugliness needed
> > > because CONCEPT_DECL is a cp/*.def attribute and so can't be mentioned
> > > in c-family/ directly; used what is used for objc method decls,
> > > an alternative would be a langhook)
> > 
> > Several of the codes in c-common.def are C++-only, you might just move it
> > over?
> > 
> > > and checks TREE_DEPRECATED in
> > > build_standard_check (not sure if that is the right spot, or whether
> > > it shouldn't be checked also for variable and function concepts and
> > > how to write testcase coverage for that).
> > 
> > I wouldn't bother with var/fn concepts, they're obsolete.
> 
> Ok, so like this?
> The previous version passed bootstrap/regtest on x86_64-linux and i686-linux,
> I'll of course test this one as well.

Better with a patch.  Sorry.

2022-11-15  Jakub Jelinek  

gcc/c-family/
* c-common.def (CONCEPT_DECL): New tree, moved here from
cp-tree.def.
* c-common.cc (c_common_init_ts): Handle CONCEPT_DECL.
* c-attribs.cc (handle_deprecated_attribute): Allow deprecated
attribute on CONCEPT_DECL.
gcc/cp/
* cp-tree.def (CONCEPT_DECL): Move to c-common.def.
* cp-objcp-common.cc (cp_common_init_ts): Don't handle CONCEPT_DECL
here.
* cp-tree.h (finish_concept_definition): Add ATTRS parameter.
* parser.cc (cp_parser_concept_definition): Parse attributes in
between identifier and =.  Adjust finish_concept_definition
caller.
* pt.cc (finish_concept_definition): Add ATTRS parameter.  Call
cplus_decl_attributes.
* constraint.cc (build_standard_check): If CONCEPT_DECL is
TREE_DEPRECATED, emit -Wdeprecated-declaration warnings.
gcc/testsuite/
* g++.dg/cpp2a/concepts-dr2428.C: New test.

--- gcc/c-family/c-common.def.jj2022-10-14 09:28:27.975164491 +0200
+++ gcc/c-family/c-common.def   2022-11-15 09:34:01.384591076 +0100
@@ -81,6 +81,14 @@ DEFTREECODE (CONTINUE_STMT, "continue_st
SWITCH_STMT_SCOPE, respectively.  */
 DEFTREECODE (SWITCH_STMT, "switch_stmt", tcc_statement, 4)
 
+/* Extensions for C++ Concepts. */
+
+/* Concept definition. This is not entirely different than a VAR_DECL
+   except that a) it must be a template, and b) doesn't have the wide
+   range of value and linkage options available to variables.  Used
+   by C++ FE and in c-family attribute handling.  */
+DEFTREECODE (CONCEPT_DECL, "concept_decl", tcc_declaration, 0)
+
 /*
 Local variables:
 mode:c
--- gcc/c-family/c-common.cc.jj 2022-11-13 12:29:08.165504692 +0100
+++ gcc/c-family/c-common.cc2022-11-15 09:34:48.828950083 +0100
@@ -8497,6 +8497,8 @@ c_common_init_ts (void)
   MARK_TS_EXP (FOR_STMT);
   MARK_TS_EXP (SWITCH_STMT);
   MARK_TS_EXP (WHILE_STMT);
+
+  MARK_TS_DECL_COMMON (CONCEPT_DECL);
 }
 
 /* Build a user-defined numeric literal out of an integer constant type VALUE
--- gcc/c-family/c-attribs.cc.jj2022-11-14 13:35:34.184160348 +0100
+++ gcc/c-family/c-attribs.cc   2022-11-15 09:30:57.370081060 +0100
@@ -4211,7 +4211,8 @@ handle_deprecated_attribute (tree *node,
  || VAR_OR_FUNCTION_DECL_P (decl)
  || TREE_CODE (decl) == FIELD_DECL
  || TREE_CODE (decl) == CONST_DECL
- || objc_method_decl (TREE_CODE (decl)))
+ || objc_method_decl (TREE_CODE (decl))
+ || TREE_CODE (decl) == CONCEPT_DECL)
TREE_DEPRECATED (decl) = 1;
   else if (TREE_CODE (decl) == LABEL_DECL)
{
--- gcc/cp/cp-tree.def.jj   2022-09-29 18:11:34.83800 +0200
+++ gcc/cp/cp-tree.def  2022-11-15 09:32:17.456996090 +0100
@@ -495,11 +495,6 @@ DEFTREECODE (OMP_DEPOBJ, "omp_depobj", t
 
 /* Extensions for Concepts. */
 
-/* Concept definition. This is not entirely different than a VAR_DECL
-   except that a) it must be a template, and b) doesn't have the wide
-   range of value and linkage options available to variables.  */
-DEFTREECODE (CONCEPT_DECL, "concept_decl", tcc_declaration, 0)
-
 /* Used to represent information associated with constrained declarations. */
 DEFTREECODE (CONSTRAINT_INFO, "constraint_info", tcc_exceptional, 0)
 
--- gcc/cp/cp-objcp-common.cc.jj2022-09-30 18:38:55.349607203 +0200
+++ gcc/cp/cp-objcp-common.cc   2022-11-15 09:34:21.963313049 +0100
@@ -473,7 +473,6 @@ cp_common_init_ts (void)
   /* New decls.  */
   MARK_TS_DECL_COMMON (TEMPLATE_DECL);
   MARK_TS_DECL_COMMON (WILDCARD_DECL);
-  MARK_TS_DECL_COMMON (CONCEPT_DECL);
 
   MARK_TS_DECL_NON_COMMON (USING_DECL);
 
--- gcc/cp/cp-tree.h.jj 2022-11-15 08:17:07.561388452 +0100
+++ gcc/cp/cp-tree.h2022-11-15 09:30:57.371081046 +0100
@@ -8324,7 +8324,7 @@ struct diagnosing_failed_constraint
 extern cp_expr finish_constraint_or_expr   (location_t, cp_expr, cp_expr);

Re: [PATCH v2] LoongArch: Add prefetch instructions.

[Xi Ruoyao via Gcc-patches]

On Sat, 2022-11-12 at 17:45 +0800, Xi Ruoyao via Gcc-patches wrote:
> void prefetch(char *ptr, int off)
> {
> return __builtin_prefetch(ptr + off);
> }
> 
> It's compiled to "preldx 0,$r4,$r5".  I don't think it's correct
> because
> according to the doc, rk should contains several bit-fields instead of
> an offset.

Hi Lulu,

Considering we are in stage 3 now and we can still push patches which
have been reviewed (in the first week of stage 3), I guess we can add
preld for GCC 13 and try preldx in the next development cycle?

BTW if preldx behaves exactly as how the manual says, I think it's not
possible to invoke it correctly in GNU C unless using inline assembly...

-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University

Re: [PATCH v2] LoongArch: Add prefetch instructions.

[Lulu Cheng]

在 2022/11/15 下午5:17, Xi Ruoyao 写道:

On Sat, 2022-11-12 at 17:45 +0800, Xi Ruoyao via Gcc-patches wrote:

void prefetch(char *ptr, int off)
{
 return __builtin_prefetch(ptr + off);
}

It's compiled to "preldx 0,$r4,$r5".  I don't think it's correct
because
according to the doc, rk should contains several bit-fields instead of
an offset.

Hi Lulu,

Considering we are in stage 3 now and we can still push patches which
have been reviewed (in the first week of stage 3), I guess we can add
preld for GCC 13 and try preldx in the next development cycle?

BTW if preldx behaves exactly as how the manual says, I think it's not
possible to invoke it correctly in GNU C unless using inline assembly...

Well, I also want to add preld and instant load optimization support in 
this release.


I will send patch in the next two days:-)

Re: [PATCH 3/8]middle-end: Support extractions of subvectors from arbitrary element position inside a vector

[Hongtao Liu via Gcc-patches]

On Tue, Nov 15, 2022 at 4:51 PM Tamar Christina  wrote:
>
> > -Original Message-
> > From: Hongtao Liu 
> > Sent: Tuesday, November 15, 2022 8:36 AM
> > To: Tamar Christina 
> > Cc: Richard Sandiford ; Tamar Christina via
> > Gcc-patches ; nd ;
> > rguent...@suse.de
> > Subject: Re: [PATCH 3/8]middle-end: Support extractions of subvectors from
> > arbitrary element position inside a vector
> >
> > Hi:
> >   I'm from https://gcc.gnu.org/pipermail/gcc-patches/2022-
> > November/606040.html.
> > >  }
> > >
> > >/* See if we can get a better vector mode before extracting.  */
> > > diff --git a/gcc/optabs.cc b/gcc/optabs.cc index
> > >
> > cff37ccb0dfc3dd79b97d0abfd872f340855dc96..f338df410265dfe55b689616009
> > 0
> > > a453cc6a28d9 100644
> > > --- a/gcc/optabs.cc
> > > +++ b/gcc/optabs.cc
> > > @@ -6267,6 +6267,7 @@ expand_vec_perm_const (machine_mode mode,
> > rtx v0, rtx v1,
> > >v0_qi = gen_lowpart (qimode, v0);
> > >v1_qi = gen_lowpart (qimode, v1);
> > >if (targetm.vectorize.vec_perm_const != NULL
> > > + && targetm.can_change_mode_class (mode, qimode, ALL_REGS)
> > It looks like you want to guard gen_lowpart, shouldn't it be better to use
> > validate_subreg  or (tmp = gen_lowpart_if_possible (mode, target_qi)).
> > IMHO, targetm.can_change_mode_class is mostly used for RA, but not to
> > guard gen_lowpart.
>
> Hmm I don't think this is quite true, there are existing usages in expr.cc 
> and rtanal.cc
> That do this and aren't part of RA.  As I mentioned before for instance the
> canoncalization of vec_select to subreg in rtlanal for instances uses this.
In theory, we need to iterate through all reg classes that can be
assigned for both qimode and mode, if any regclass returns true for
targetm.can_change_mode_class, the bitcast(validate_subreg) should be
ok.
Here we just passed ALL_REGS.
>
> So there are already existing precedence for this.  And the documentation for
> the hook says:
>
> "This hook returns true if it is possible to bitcast values held in registers 
> of class rclass from mode from to mode to and if doing so preserves the 
> low-order bits that are common to both modes. The result is only meaningful 
> if rclass has registers that can hold both from and to. The default 
> implementation returns true"
>
> So it looks like it's use outside of RA is perfectly valid.. and the 
> documentation also mentions
> in the example the use from the mid-end as an example.
>
> But if the mid-end maintainers are happy I'll use something else.
>
> Tamar
>
> > I did similar things in
> > https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579296.html
> > (and ALL_REGS doesn't cover all cases for registers which are both available
> > for qimode and mode, ALL_REGS fail doesn't mean it can't be subreg, it just
> > means parts of ALL_REGS can't be subreg. but with a subset of ALL_REGS,
> > there could be a reg class which return true for
> > targetm.can_change_mode_class)
> > >   && targetm.vectorize.vec_perm_const (qimode, qimode, target_qi,
> > v0_qi,
> > >v1_qi, qimode_indices))
> > > return gen_lowpart (mode, target_qi); @@ -6311,7 +6312,8 @@
> > > expand_vec_perm_const (machine_mode mode, rtx v0, rtx v1,
> > >  }
> > >
> > >if (qimode != VOIDmode
> > > -  && selector_fits_mode_p (qimode, qimode_indices))
> > > +  && selector_fits_mode_p (qimode, qimode_indices)
> > > +  && targetm.can_change_mode_class (mode, qimode, ALL_REGS))
> > >  {
> > >icode = direct_optab_handler (vec_perm_optab, qimode);
> > >if (icode != CODE_FOR_nothing)
> > > diff --git a/gcc/testsuite/gcc.target/aarch64/ext_1.c
> > > b/gcc/testsuite/gcc.target/aarch64/ext_1.c
> > > new file mode 100644
> > > index
> > >
> > ..18a10a14f1161584267a8472e5
> > 71
> > > b3bc2ddf887a
> >
> >
> >
> >
> > --
> > BR,
> > Hongtao



-- 
BR,
Hongtao

RE: [PATCH 3/8]middle-end: Support extractions of subvectors from arbitrary element position inside a vector

[Tamar Christina via Gcc-patches]

> -Original Message-
> From: Hongtao Liu 
> Sent: Tuesday, November 15, 2022 9:37 AM
> To: Tamar Christina 
> Cc: Richard Sandiford ; Tamar Christina via
> Gcc-patches ; nd ;
> rguent...@suse.de
> Subject: Re: [PATCH 3/8]middle-end: Support extractions of subvectors from
> arbitrary element position inside a vector
> 
> On Tue, Nov 15, 2022 at 4:51 PM Tamar Christina
>  wrote:
> >
> > > -Original Message-
> > > From: Hongtao Liu 
> > > Sent: Tuesday, November 15, 2022 8:36 AM
> > > To: Tamar Christina 
> > > Cc: Richard Sandiford ; Tamar Christina
> > > via Gcc-patches ; nd ;
> > > rguent...@suse.de
> > > Subject: Re: [PATCH 3/8]middle-end: Support extractions of
> > > subvectors from arbitrary element position inside a vector
> > >
> > > Hi:
> > >   I'm from https://gcc.gnu.org/pipermail/gcc-patches/2022-
> > > November/606040.html.
> > > >  }
> > > >
> > > >/* See if we can get a better vector mode before extracting.
> > > > */ diff --git a/gcc/optabs.cc b/gcc/optabs.cc index
> > > >
> > >
> cff37ccb0dfc3dd79b97d0abfd872f340855dc96..f338df410265dfe55b68961600
> > > 9
> > > 0
> > > > a453cc6a28d9 100644
> > > > --- a/gcc/optabs.cc
> > > > +++ b/gcc/optabs.cc
> > > > @@ -6267,6 +6267,7 @@ expand_vec_perm_const (machine_mode
> mode,
> > > rtx v0, rtx v1,
> > > >v0_qi = gen_lowpart (qimode, v0);
> > > >v1_qi = gen_lowpart (qimode, v1);
> > > >if (targetm.vectorize.vec_perm_const != NULL
> > > > + && targetm.can_change_mode_class (mode, qimode,
> > > > + ALL_REGS)
> > > It looks like you want to guard gen_lowpart, shouldn't it be better
> > > to use validate_subreg  or (tmp = gen_lowpart_if_possible (mode,
> target_qi)).
> > > IMHO, targetm.can_change_mode_class is mostly used for RA, but not
> > > to guard gen_lowpart.
> >
> > Hmm I don't think this is quite true, there are existing usages in
> > expr.cc and rtanal.cc That do this and aren't part of RA.  As I
> > mentioned before for instance the canoncalization of vec_select to subreg
> in rtlanal for instances uses this.
> In theory, we need to iterate through all reg classes that can be assigned for
> both qimode and mode, if any regclass returns true for
> targetm.can_change_mode_class, the bitcast(validate_subreg) should be ok.
> Here we just passed ALL_REGS.

Yes, and most targets where this transformation is valid return true here.

I've checked:
 * alpha
 * arm
 * aarch64
 * rs6000
 * s390
 * sparc
 * pa
 * mips

And even the default example that other targets use from the documentation
would return true as the size of the modes are the same.

X86 and RISCV are the only two targets that I found (but didn't check all) that
blankly return a result based on just the register classes.

That is to say, there are more targets that adhere to the interpretation that
rclass here means "should be possible in some class in rclass" rather than
"should be possible in ALL classes of rclass".

> >
> > So there are already existing precedence for this.  And the
> > documentation for the hook says:
> >
> > "This hook returns true if it is possible to bitcast values held in 
> > registers of
> class rclass from mode from to mode to and if doing so preserves the low-
> order bits that are common to both modes. The result is only meaningful if
> rclass has registers that can hold both from and to. The default
> implementation returns true"
> >
> > So it looks like it's use outside of RA is perfectly valid.. and the
> > documentation also mentions in the example the use from the mid-end as
> an example.
> >
> > But if the mid-end maintainers are happy I'll use something else.
> >
> > Tamar
> >
> > > I did similar things in
> > > https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579296.html
> > > (and ALL_REGS doesn't cover all cases for registers which are both
> > > available for qimode and mode, ALL_REGS fail doesn't mean it can't
> > > be subreg, it just means parts of ALL_REGS can't be subreg. but with
> > > a subset of ALL_REGS, there could be a reg class which return true
> > > for
> > > targetm.can_change_mode_class)
> > > >   && targetm.vectorize.vec_perm_const (qimode, qimode,
> > > > target_qi,
> > > v0_qi,
> > > >v1_qi, qimode_indices))
> > > > return gen_lowpart (mode, target_qi); @@ -6311,7 +6312,8
> > > > @@ expand_vec_perm_const (machine_mode mode, rtx v0, rtx v1,
> > > >  }
> > > >
> > > >if (qimode != VOIDmode
> > > > -  && selector_fits_mode_p (qimode, qimode_indices))
> > > > +  && selector_fits_mode_p (qimode, qimode_indices)
> > > > +  && targetm.can_change_mode_class (mode, qimode, ALL_REGS))
> > > >  {
> > > >icode = direct_optab_handler (vec_perm_optab, qimode);
> > > >if (icode != CODE_FOR_nothing) diff --git
> > > > a/gcc/testsuite/gcc.target/aarch64/ext_1.c
> > > > b/gcc/testsuite/gcc.target/aarch64/ext_1.c
> > > > new file mode 100644
> > > > index
> > > >
> > >
> 000

Re: GCC 13.0.0 Status Report (2022-11-14), Stage 3 in effect now

[Martin Liška]

On 11/14/22 18:21, Xi Ruoyao wrote:
> Hi Martin,
> 

Hello.

> Is it allowed to merge libsanitizer from LLVM in stage 3?  If not I'd
> like to cherry pick some commits from LLVM [to fix some stupid errors
> I've made in LoongArch libasan :(].

I'm sorry but I was really busy with the porting of the documentation to Sphinx.

Anyway, yes, we should make one one libsanitizer merge, but RM should likely
approve it: Richi, Jakub, do you support it?

Thanks,
Martin

> 
> On Mon, 2022-11-14 at 13:21 +, Richard Biener via Gcc-patches wrote:
>> Status
>> ==
>>
>> The GCC development branch which will become GCC 13 is now in
>> bugfixing mode (Stage 3) until the end of Jan 15th.
>>
>> As usual the first weeks of Stage 3 are used to feature patches
>> posted late during Stage 1.  At some point unreviewed features
>> need to be postponed for the next Stage 1.
>>
>>
>> Quality Data
>> 
>>
>> Priority  #   Change from last report
>>     ---   ---
>> P1  33    
>> P2  473 
>> P3  113   +  29
>> P4  253   +   6
>> P5  25   
>>     ---   ---
>> Total P1-P3 619   +  29
>> Total   897   +  35
>>
>>
>> Previous Report
>> ===
>>
>> https://gcc.gnu.org/pipermail/gcc/2022-October/239690.html
> 

Re: Revert Sphinx documentation [Was: Issues with Sphinx]

[Martin Liška]

On 11/14/22 14:06, Gerald Pfeifer wrote:
> On Mon, 14 Nov 2022, Martin Liška wrote:
>> The situation with the Sphinx migration went out of control. The TODO 
>> list overwhelmed me and there are road-blocks that can't be easily fixed 
>> with what Sphinx currently supports.
> 
> This migration was/is a huge and complex undertaking, and you have been 
> patiently chipping away at obstacle after obstacle.
> 
> So while it probably is disappointing it did not go through this time,
> you made a lot of progress and important contributions - and we all 
> learned quite a bit more, also in terms of (not so obvious) requirements,
> dependencies, and road blocks left which you summarized.

Hello.

I see it similarly and I'm trying to take the best out of it.

> 
> 
> Timing was tricky for me being on the road last week and I am definitely
> committed to keep helping with this transition. Maybe soon after we are in 
> stage 1 again?

For now, I'm not planning working on that in the future. Maybe, someone else
can carry on working on that.

> 
> And would it make sense to convert at least our installation docs and
> https://gcc.gnu.org/install/ for the GCC 13 release?

Yes, it's definitely doable as it's quite small, not split among too many .rst
files and the HTML version is mainly built at our server in order to populate
gcc.gnu.org/install

I'm willing to help anybody, but it won't be me who will suggest/send
the patches.

Thanks for understanding.
Martin

> 
> Gerald

Re: GCC 13.0.0 Status Report (2022-11-14), Stage 3 in effect now

[Jakub Jelinek via Gcc-patches]

On Tue, Nov 15, 2022 at 11:02:53AM +0100, Martin Liška wrote:
> > Is it allowed to merge libsanitizer from LLVM in stage 3?  If not I'd
> > like to cherry pick some commits from LLVM [to fix some stupid errors
> > I've made in LoongArch libasan :(].
> 
> I'm sorry but I was really busy with the porting of the documentation to 
> Sphinx.
> 
> Anyway, yes, we should make one one libsanitizer merge, but RM should likely
> approve it: Richi, Jakub, do you support it?

Could you please prepare a patch, so that we can see how much actually
changed and decide based on that whether to go for a merge or cherry-picking
one or more commits?
I think last merge was done by you at the end of August, so we have
2.5 months of changes to potentially merge.

Jakub

Re: Rust frontend patches v3

[Arthur Cohen]

Hi Richard,

On 11/10/22 11:52, Richard Biener wrote:

On Wed, Oct 26, 2022 at 10:16 AM  wrote:


This is the fixed version of our previous patch set for gccrs - We've adressed
the comments raised in our previous emails.

This patch set does not contain any work that was not previously included, such
as closure support, the constant evaluator port, or the better implementation
of target hooks by Iain Buclaw. They will follow up in subsequent patch sets.

Thanks again to Open Source Security, inc and Embecosm who have accompanied us
for this work.

Many thanks to all of the contributors and our community, who made this
possible.

A very special thanks to Philip Herron, without whose mentoring I would have
never been in a position to send these patches.

You can see the current status of our work on our branch:
https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/heads/devel/rust/master

The patch set contains the following:


Can you mark the patches that have been reviewed/approved?  Can you
maybe either split the series or organize it in a way to separate the
pieces touching common parts of GCC from the gcc/rust/ parts?
Can you separate testsuite infrastructure from actual tests, can
you mark/separate target specific changes?  And for those (then small)
changes CC the appropriate maintainers?


Thanks a lot for all the feedback. I'll apply the required changes and 
make sure the patchset(s) are a bit easier to review.


All the best,

Arthur



Thanks,
Richard.


[PATCH Rust front-end v3 01/46] Use DW_ATE_UTF for the Rust 'char'
[PATCH Rust front-end v3 02/46] gccrs: Add nessecary hooks for a Rust
[PATCH Rust front-end v3 03/46] gccrs: Add Debug info testsuite
[PATCH Rust front-end v3 04/46] gccrs: Add link cases testsuite
[PATCH Rust front-end v3 05/46] gccrs: Add general compilation test
[PATCH Rust front-end v3 06/46] gccrs: Add execution test cases
[PATCH Rust front-end v3 07/46] gccrs: Add gcc-check-target
[PATCH Rust front-end v3 08/46] gccrs: Add Rust front-end base AST
[PATCH Rust front-end v3 09/46] gccrs: Add definitions of Rust Items
[PATCH Rust front-end v3 10/46] gccrs: Add full definitions of Rust
[PATCH Rust front-end v3 11/46] gccrs: Add Rust AST visitors
[PATCH Rust front-end v3 12/46] gccrs: Add Lexer for Rust front-end
[PATCH Rust front-end v3 13/46] gccrs: Add Parser for Rust front-end
[PATCH Rust front-end v3 14/46] gccrs: Add Parser for Rust front-end
[PATCH Rust front-end v3 15/46] gccrs: Add expansion pass for the
[PATCH Rust front-end v3 16/46] gccrs: Add name resolution pass to
[PATCH Rust front-end v3 17/46] gccrs: Add declarations for Rust HIR
[PATCH Rust front-end v3 18/46] gccrs: Add HIR definitions and
[PATCH Rust front-end v3 19/46] gccrs: Add AST to HIR lowering pass
[PATCH Rust front-end v3 20/46] gccrs: Add wrapper for make_unique
[PATCH Rust front-end v3 21/46] gccrs: Add port of FNV hash used
[PATCH Rust front-end v3 22/46] gccrs: Add Rust ABI enum helpers
[PATCH Rust front-end v3 23/46] gccrs: Add Base62 implementation
[PATCH Rust front-end v3 24/46] gccrs: Add implementation of Optional
[PATCH Rust front-end v3 25/46] gccrs: Add attributes checker
[PATCH Rust front-end v3 26/46] gccrs: Add helpers mappings canonical
[PATCH Rust front-end v3 27/46] gccrs: Add type resolution and trait
[PATCH Rust front-end v3 28/46] gccrs: Add Rust type information
[PATCH Rust front-end v3 29/46] gccrs: Add remaining type system
[PATCH Rust front-end v3 30/46] gccrs: Add unsafe checks for Rust
[PATCH Rust front-end v3 31/46] gccrs: Add const checker
[PATCH Rust front-end v3 32/46] gccrs: Add privacy checks
[PATCH Rust front-end v3 33/46] gccrs: Add dead code scan on HIR
[PATCH Rust front-end v3 34/46] gccrs: Add unused variable scan
[PATCH Rust front-end v3 35/46] gccrs: Add metadata ouptput pass
[PATCH Rust front-end v3 36/46] gccrs: Add base for HIR to GCC
[PATCH Rust front-end v3 37/46] gccrs: Add HIR to GCC GENERIC
[PATCH Rust front-end v3 38/46] gccrs: Add HIR to GCC GENERIC
[PATCH Rust front-end v3 39/46] gccrs: These are wrappers ported from
[PATCH Rust front-end v3 40/46] gccrs: Add GCC Rust front-end
[PATCH Rust front-end v3 41/46] gccrs: Add config-lang.in
[PATCH Rust front-end v3 42/46] gccrs: Add lang-spec.h
[PATCH Rust front-end v3 43/46] gccrs: Add lang.opt
[PATCH Rust front-end v3 44/46] gccrs: Add compiler driver
[PATCH Rust front-end v3 45/46] gccrs: Compiler proper interface
[PATCH Rust front-end v3 46/46] gccrs: Add README, CONTRIBUTING and



--
Arthur Cohen 

Toolchain Engineer

Embecosm GmbH

Geschäftsführer: Jeremy Bennett
Niederlassung: Nürnberg
Handelsregister: HR-B 36368
www.embecosm.de

Fürther Str. 27
90429 Nürnberg


Tel.: 091 - 128 707 040
Fax: 091 - 128 707 077


OpenPGP_0x1B3465B044AD9C65.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

[PATCH]middle-end: replace GET_MODE_WIDER_MODE with GET_MODE_NEXT_MODE

[Tamar Christina via Gcc-patches]

Hi All,

After the fix to the addsub patch yesterday for bootstrap I had only regtested 
on x86.
While looking today it seemed the new tests were failing, this was caused
by a change in the behavior of the GET_MODE_WIDER_MODE macro on trunk.

This patch fixes that issue. Sorry for the mess, have rebased all branches now.

Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.

Ok for master?

Thanks,
Tamar

gcc/ChangeLog:

* match.pd: Replace GET_MODE_WIDER_MODE with
GET_MODE_NEXT_MODE.

--- inline copy of patch -- 
diff --git a/gcc/match.pd b/gcc/match.pd
index 
1b0ab7cf60fa4772fbe8304c622b0b8fab1bdefa..28191a992039c6f3a1dab5f7c0e35dd58dc47092
 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -7997,7 +7997,7 @@ and,
machine_mode wide_mode;
  }
  (if (sel.series_p (0, 2, 0, 2)
-  && GET_MODE_WIDER_MODE (vec_mode).exists (&wide_mode)
+  && GET_MODE_NEXT_MODE (vec_mode).exists (&wide_mode)
  && VECTOR_MODE_P (wide_mode)
  && (GET_MODE_UNIT_BITSIZE (vec_mode) * 2
  == GET_MODE_UNIT_BITSIZE (wide_mode)))




-- 
diff --git a/gcc/match.pd b/gcc/match.pd
index 
1b0ab7cf60fa4772fbe8304c622b0b8fab1bdefa..28191a992039c6f3a1dab5f7c0e35dd58dc47092
 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -7997,7 +7997,7 @@ and,
machine_mode wide_mode;
  }
  (if (sel.series_p (0, 2, 0, 2)
-  && GET_MODE_WIDER_MODE (vec_mode).exists (&wide_mode)
+  && GET_MODE_NEXT_MODE (vec_mode).exists (&wide_mode)
  && VECTOR_MODE_P (wide_mode)
  && (GET_MODE_UNIT_BITSIZE (vec_mode) * 2
  == GET_MODE_UNIT_BITSIZE (wide_mode)))

Re: [PATCH] RISC-V: Optimal RVV epilogue logic.

[Philipp Tomsich]

On Mon, 14 Nov 2022 at 17:29, jiawei  wrote:
>
> Skip add insn generate if the adjust size equal to zero.
>
> gcc/ChangeLog:
>
> * config/riscv/riscv.cc (riscv_expand_epilogue):
> New if control segement.
>
> ---
>  gcc/config/riscv/riscv.cc | 18 ++
>  1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 02a01ca0b7c..af138db7545 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -5186,24 +5186,26 @@ riscv_expand_epilogue (int style)
> }
>
>/* Get an rtx for STEP1 that we can add to BASE.  */
> -  rtx adjust = GEN_INT (step1.to_constant ());
> -  if (!SMALL_OPERAND (step1.to_constant ()))
> +  if (step1.to_constant () != 0){
> +rtx adjust = GEN_INT (step1.to_constant ());
> +if (!SMALL_OPERAND (step1.to_constant ()))

Please take a look at the recent improvements for the add3
expander (recently submitted as
https://patchwork.ozlabs.org/project/gcc/patch/20221109230718.3240479-1-philipp.toms...@vrull.eu/).
Maybe you also want to use the test for the addi_operand(...) instead
of SMALL_OPERAND?

> {
>   riscv_emit_move (RISCV_PROLOGUE_TEMP (Pmode), adjust);
>   adjust = RISCV_PROLOGUE_TEMP (Pmode);
> }
>
> -  insn = emit_insn (
> +insn = emit_insn (
>gen_add3_insn (stack_pointer_rtx, stack_pointer_rtx, adjust));
>
> -  rtx dwarf = NULL_RTX;
> -  rtx cfa_adjust_rtx = gen_rtx_PLUS (Pmode, stack_pointer_rtx,
> +rtx dwarf = NULL_RTX;
> +rtx cfa_adjust_rtx = gen_rtx_PLUS (Pmode, stack_pointer_rtx,
>  GEN_INT (step2));
>
> -  dwarf = alloc_reg_note (REG_CFA_DEF_CFA, cfa_adjust_rtx, dwarf);
> -  RTX_FRAME_RELATED_P (insn) = 1;
> +dwarf = alloc_reg_note (REG_CFA_DEF_CFA, cfa_adjust_rtx, dwarf);
> +RTX_FRAME_RELATED_P (insn) = 1;
>
> -  REG_NOTES (insn) = dwarf;
> +REG_NOTES (insn) = dwarf;
> +  }
>  }
>else if (frame_pointer_needed)
>  {
> --
> 2.25.1
>

Re: [PATCH 2/2]AArch64 Support new tbranch optab.

[Richard Sandiford via Gcc-patches]

Tamar Christina  writes:
> Hello,
>
> Ping and updated patch.
>
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>
> Ok for master?
>
> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
> * config/aarch64/aarch64.md (*tb1): Rename to...
> (*tb1): ... this.
> (tbranch4): New.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/aarch64/tbz_1.c: New test.
>
> --- inline copy of patch ---
>
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 
> 2bc2684b82c35a44e0a2cea6e3aaf32d939f8cdf..d7684c93fba5b717d568e1a4fd712bde55c7c72e
>  100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -943,12 +943,29 @@ (define_insn "*cb1"
>   (const_int 1)))]
>  )
>
> -(define_insn "*tb1"
> +(define_expand "tbranch4"
>[(set (pc) (if_then_else
> - (EQL (zero_extract:DI (match_operand:GPI 0 "register_operand" 
> "r")
> -   (const_int 1)
> -   (match_operand 1
> - "aarch64_simd_shift_imm_" "n"))
> +   (match_operator 0 "aarch64_comparison_operator"
> +[(match_operand:ALLI 1 "register_operand")
> + (match_operand:ALLI 2 
> "aarch64_simd_shift_imm_")])
> +   (label_ref (match_operand 3 "" ""))
> +   (pc)))]
> +  "optimize > 0"

Why's the pattern conditional on optimize?  Seems a valid choice at -O0 too.

I think the split here shows the difficulty with having a single optab
and a comparison operator though.  operand 0 can be something like:

  (eq x 1)

but we're not comparing x for equality with 1.  We're testing whether
bit 1 is zero.  This means that operand 0 can't be taken literally
and can't be used directly in insn patterns.

In an earlier review, I'd said:

  For the TB instructions (and for other similar instructions that I've
  seen on other architectures) it would be more useful to have a single-bit
  test, with operand 4 specifying the bit position.  Arguably it might then
  be better to have separate eq and ne optabs, to avoid the awkward doubling
  of the operands (operand 1 contains operands 2 and 3).

I think we should do that eq/ne split (sorry for not pushing harder for
it before).

Thanks,
Richard



> +{
> +  rtx bitvalue = gen_reg_rtx (DImode);
> +  rtx tmp = simplify_gen_subreg (DImode, operands[1], GET_MODE 
> (operands[1]), 0);
> +  emit_insn (gen_extzv (bitvalue, tmp, const1_rtx, operands[2]));
> +  operands[2] = const0_rtx;
> +  operands[1] = aarch64_gen_compare_reg (GET_CODE (operands[0]), bitvalue,
> +operands[2]);
> +})
> +
> +(define_insn "*tb1"
> +  [(set (pc) (if_then_else
> + (EQL (zero_extract:GPI (match_operand:ALLI 0 "register_operand" 
> "r")
> +(const_int 1)
> +(match_operand 1
> +  "aarch64_simd_shift_imm_" 
> "n"))
>(const_int 0))
>  (label_ref (match_operand 2 "" ""))
>  (pc)))
> @@ -959,15 +976,15 @@ (define_insn "*tb1"
>{
> if (get_attr_far_branch (insn) == 1)
>   return aarch64_gen_far_branch (operands, 2, "Ltb",
> -"\\t%0, %1, ");
> +"\\t%0, %1, ");
> else
>   {
> operands[1] = GEN_INT (HOST_WIDE_INT_1U << UINTVAL (operands[1]));
> -   return "tst\t%0, %1\;\t%l2";
> +   return "tst\t%0, %1\;\t%l2";
>   }
>}
>  else
> -  return "\t%0, %1, %l2";
> +  return "\t%0, %1, %l2";
>}
>[(set_attr "type" "branch")
> (set (attr "length")
> diff --git a/gcc/testsuite/gcc.target/aarch64/tbz_1.c 
> b/gcc/testsuite/gcc.target/aarch64/tbz_1.c
> new file mode 100644
> index 
> ..86f5d3e23cf7f1ea6f3596549ce1a0cff6774463
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/tbz_1.c
> @@ -0,0 +1,95 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-O2 -std=c99  -fno-unwind-tables 
> -fno-asynchronous-unwind-tables" } */
> +/* { dg-final { check-function-bodies "**" "" "" { target { le } } } } */
> +
> +#include 
> +
> +void h(void);
> +
> +/*
> +** g1:
> +** tbnzx[0-9]+, #?0, .L([0-9]+)
> +** ret
> +** ...
> +*/
> +void g1(bool x)
> +{
> +  if (__builtin_expect (x, 0))
> +h ();
> +}
> +
> +/*
> +** g2:
> +** tbz x[0-9]+, #?0, .L([0-9]+)
> +** b   h
> +** ...
> +*/
> +void g2(bool x)
> +{
> +  if (__builtin_expect (x, 1))
> +h ();
> +}
> +
> +/*
> +** g3_ge:
> +** tbnzw[0-9]+, #?31, .L[0-9]+
> +** b   h
> +** ...
> +*/
> +void g3_ge(int x)
> +{
> +  if (__builtin_expect (x >= 0, 1))
> +h ();
> +}
> +
> +/*
> +** g3_gt:
> +** cmp w[0-9]+, 0
> +** ble .L[0-9]+
> +** b   h
> +** ...
> +*

RE: [PATCH 2/2]AArch64 Support new tbranch optab.

[Tamar Christina via Gcc-patches]

> -Original Message-
> From: Richard Sandiford 
> Sent: Tuesday, November 15, 2022 10:36 AM
> To: Tamar Christina 
> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
> ; nd ; Marcus Shawcroft
> 
> Subject: Re: [PATCH 2/2]AArch64 Support new tbranch optab.
> 
> Tamar Christina  writes:
> > Hello,
> >
> > Ping and updated patch.
> >
> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> >
> > Ok for master?
> >
> > Thanks,
> > Tamar
> >
> > gcc/ChangeLog:
> >
> > * config/aarch64/aarch64.md (*tb1): Rename to...
> > (*tb1): ... this.
> > (tbranch4): New.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.target/aarch64/tbz_1.c: New test.
> >
> > --- inline copy of patch ---
> >
> > diff --git a/gcc/config/aarch64/aarch64.md
> > b/gcc/config/aarch64/aarch64.md index
> >
> 2bc2684b82c35a44e0a2cea6e3aaf32d939f8cdf..d7684c93fba5b717d568e1a4fd
> 71
> > 2bde55c7c72e 100644
> > --- a/gcc/config/aarch64/aarch64.md
> > +++ b/gcc/config/aarch64/aarch64.md
> > @@ -943,12 +943,29 @@ (define_insn "*cb1"
> >   (const_int 1)))]
> >  )
> >
> > -(define_insn "*tb1"
> > +(define_expand "tbranch4"
> >[(set (pc) (if_then_else
> > - (EQL (zero_extract:DI (match_operand:GPI 0 "register_operand"
> "r")
> > -   (const_int 1)
> > -   (match_operand 1
> > - "aarch64_simd_shift_imm_" "n"))
> > +   (match_operator 0 "aarch64_comparison_operator"
> > +[(match_operand:ALLI 1 "register_operand")
> > + (match_operand:ALLI 2
> "aarch64_simd_shift_imm_")])
> > +   (label_ref (match_operand 3 "" ""))
> > +   (pc)))]
> > +  "optimize > 0"
> 
> Why's the pattern conditional on optimize?  Seems a valid choice at -O0 too.
> 

Hi,

I had explained the reason why in the original patch, just didn't repeat it in 
the ping:

Instead of emitting the instruction directly I've chosen to expand the pattern 
using a zero extract and generating the existing pattern for comparisons for two
reasons:

  1. Allows for CSE of the actual comparison.
  2. It looks like the code in expand makes the label as unused and removed it
 if it doesn't see a separate reference to it.

Because of this expansion though I disable the pattern at -O0 since we have no 
combine in that case so we'd end up with worse code.  I did try emitting the 
pattern directly, but as mentioned in no#2 expand would then kill the label.

Basically I emit the pattern directly, immediately during expand the label is 
marked as dead for some weird reason.

Tamar.

> I think the split here shows the difficulty with having a single optab and a
> comparison operator though.  operand 0 can be something like:
> 
>   (eq x 1)
> 
> but we're not comparing x for equality with 1.  We're testing whether bit 1 is
> zero.  This means that operand 0 can't be taken literally and can't be used
> directly in insn patterns.
> 
> In an earlier review, I'd said:
> 
>   For the TB instructions (and for other similar instructions that I've
>   seen on other architectures) it would be more useful to have a single-bit
>   test, with operand 4 specifying the bit position.  Arguably it might then
>   be better to have separate eq and ne optabs, to avoid the awkward
> doubling
>   of the operands (operand 1 contains operands 2 and 3).
> 
> I think we should do that eq/ne split (sorry for not pushing harder for it
> before).
> 
> Thanks,
> Richard
> 
> 
> 
> > +{
> > +  rtx bitvalue = gen_reg_rtx (DImode);
> > +  rtx tmp = simplify_gen_subreg (DImode, operands[1], GET_MODE
> > +(operands[1]), 0);
> > +  emit_insn (gen_extzv (bitvalue, tmp, const1_rtx, operands[2]));
> > +  operands[2] = const0_rtx;
> > +  operands[1] = aarch64_gen_compare_reg (GET_CODE (operands[0]),
> bitvalue,
> > +operands[2]);
> > +})
> > +
> > +(define_insn "*tb1"
> > +  [(set (pc) (if_then_else
> > + (EQL (zero_extract:GPI (match_operand:ALLI 0 
> > "register_operand"
> "r")
> > +(const_int 1)
> > +(match_operand 1
> > +
> > +"aarch64_simd_shift_imm_" "n"))
> >(const_int 0))
> >  (label_ref (match_operand 2 "" ""))
> >  (pc)))
> > @@ -959,15 +976,15 @@ (define_insn "*tb1"
> >{
> > if (get_attr_far_branch (insn) == 1)
> >   return aarch64_gen_far_branch (operands, 2, "Ltb",
> > -"\\t%0, %1, ");
> > +"\\t%0, %1,
> > + ");
> > else
> >   {
> > operands[1] = GEN_INT (HOST_WIDE_INT_1U << UINTVAL
> (operands[1]));
> > -   return "tst\t%0, %1\;\t%l2";
> > +   return "tst\t%0, %1\;\t%l2";
> >   }
> >}
> >  else
> > -  return "\t%0, %1, %l2";
> > +  return "\t%0, %1, %l2";

Re: [PATCH v2] aarch64: Add support for Ampere-1A (-mcpu=ampere1a) CPU

[Richard Sandiford via Gcc-patches]

Philipp Tomsich  writes:
> Richard,
>
> is this OK for backport to GCC-12 and GCC-11?

The fusion part seems potentially risky for a stable branch, but since
it's conditional on the new flag (and thus new CPU), I think it should
be OK.

So yeah, OK for both, thanks.

Richard

> Thanks,
> Philipp.
>
> On Mon, 14 Nov 2022 at 14:53, Philipp Tomsich  
> wrote:
>>
>> This patch adds support for Ampere-1A CPU:
>>  - recognize the name of the core and provide detection for -mcpu=native,
>>  - updated extra_costs,
>>  - adds a new fusion pair for (A+B+1 and A-B-1).
>>
>> Ampere-1A and Ampere-1 have more timing difference than the extra
>> costs indicate, but these don't propagate through to the headline
>> items in our extra costs (e.g. the change in latency for scalar sqrt
>> doesn't have a corresponding table entry).
>>
>> gcc/ChangeLog:
>>
>> * config/aarch64/aarch64-cores.def (AARCH64_CORE): Add ampere1a.
>> * config/aarch64/aarch64-cost-tables.h: Add ampere1a_extra_costs.
>> * config/aarch64/aarch64-fusion-pairs.def (AARCH64_FUSION_PAIR):
>> Define a new fusion pair for A+B+1/A-B-1 (i.e., add/subtract two
>> registers and then +1/-1).
>> * config/aarch64/aarch64-tune.md: Regenerate.
>> * config/aarch64/aarch64.cc (aarch_macro_fusion_pair_p): Implement
>> idiom-matcher for the new fusion pair.
>> * doc/invoke.texi: Add ampere1a.
>>
>> Signed-off-by: Philipp Tomsich 
>> ---
>>
>> Changes in v2:
>> - break line in fusion matcher to stay below 80 characters
>> - rename fusion pair addsub_2reg_const1
>> - document 'ampere1a' in invoke.texi
>>
>>  gcc/config/aarch64/aarch64-cores.def|   1 +
>>  gcc/config/aarch64/aarch64-cost-tables.h| 107 
>>  gcc/config/aarch64/aarch64-fusion-pairs.def |   1 +
>>  gcc/config/aarch64/aarch64-tune.md  |   2 +-
>>  gcc/config/aarch64/aarch64.cc   |  64 
>>  gcc/doc/invoke.texi |   2 +-
>>  6 files changed, 175 insertions(+), 2 deletions(-)
>>
>> diff --git a/gcc/config/aarch64/aarch64-cores.def 
>> b/gcc/config/aarch64/aarch64-cores.def
>> index d2671778928..aead587cec1 100644
>> --- a/gcc/config/aarch64/aarch64-cores.def
>> +++ b/gcc/config/aarch64/aarch64-cores.def
>> @@ -70,6 +70,7 @@ AARCH64_CORE("thunderxt83",   thunderxt83,   thunderx,  
>> V8A,  (CRC, CRYPTO), thu
>>
>>  /* Ampere Computing ('\xC0') cores. */
>>  AARCH64_CORE("ampere1", ampere1, cortexa57, V8_6A, (F16, RNG, AES, SHA3), 
>> ampere1, 0xC0, 0xac3, -1)
>> +AARCH64_CORE("ampere1a", ampere1a, cortexa57, V8_6A, (F16, RNG, AES, SHA3, 
>> MEMTAG), ampere1a, 0xC0, 0xac4, -1)
>>  /* Do not swap around "emag" and "xgene1",
>> this order is required to handle variant correctly. */
>>  AARCH64_CORE("emag",emag,  xgene1,V8A,  (CRC, CRYPTO), 
>> emag, 0x50, 0x000, 3)
>> diff --git a/gcc/config/aarch64/aarch64-cost-tables.h 
>> b/gcc/config/aarch64/aarch64-cost-tables.h
>> index 760d7b30368..48522606fbe 100644
>> --- a/gcc/config/aarch64/aarch64-cost-tables.h
>> +++ b/gcc/config/aarch64/aarch64-cost-tables.h
>> @@ -775,4 +775,111 @@ const struct cpu_cost_table ampere1_extra_costs =
>>}
>>  };
>>
>> +const struct cpu_cost_table ampere1a_extra_costs =
>> +{
>> +  /* ALU */
>> +  {
>> +0, /* arith.  */
>> +0, /* logical.  */
>> +0, /* shift.  */
>> +COSTS_N_INSNS (1), /* shift_reg.  */
>> +0, /* arith_shift.  */
>> +COSTS_N_INSNS (1), /* arith_shift_reg.  */
>> +0, /* log_shift.  */
>> +COSTS_N_INSNS (1), /* log_shift_reg.  */
>> +0, /* extend.  */
>> +COSTS_N_INSNS (1), /* extend_arith.  */
>> +0, /* bfi.  */
>> +0, /* bfx.  */
>> +0, /* clz.  */
>> +0, /* rev.  */
>> +0, /* non_exec.  */
>> +true   /* non_exec_costs_exec.  */
>> +  },
>> +  {
>> +/* MULT SImode */
>> +{
>> +  COSTS_N_INSNS (3),   /* simple.  */
>> +  COSTS_N_INSNS (3),   /* flag_setting.  */
>> +  COSTS_N_INSNS (3),   /* extend.  */
>> +  COSTS_N_INSNS (4),   /* add.  */
>> +  COSTS_N_INSNS (4),   /* extend_add.  */
>> +  COSTS_N_INSNS (19)   /* idiv.  */
>> +},
>> +/* MULT DImode */
>> +{
>> +  COSTS_N_INSNS (3),   /* simple.  */
>> +  0,   /* flag_setting (N/A).  */
>> +  COSTS_N_INSNS (3),   /* extend.  */
>> +  COSTS_N_INSNS (4),   /* add.  */
>> +  COSTS_N_INSNS (4),   /* extend_add.  */
>> +  COSTS_N_INSNS (35)   /* idiv.  */
>> +}
>> +  },
>> +  /* LD/ST */
>> +  {
>> +COSTS_N_INSNS (4), /* load.  */
>> +COSTS_N_INSNS (4), /* load_sign_extend.  */
>> +0, /* ldrd (n/a).  */
>> +0, /* ldm_1st.  */
>> +0,

PING: [PATCH v6] tree-optimization/101186 - extend FRE with "equivalence map" for condition prediction

[Di Zhao OS via Gcc-patches]

Hi,

I saw that Stage 1 of GCC 13 development is just ended. So is this
considered? Or should I bring this up when general development is
reopened?


Thanks,
Di Zhao


> -Original Message-
> From: Di Zhao OS
> Sent: Tuesday, October 25, 2022 8:18 AM
> To: gcc-patches@gcc.gnu.org
> Cc: Richard Biener 
> Subject: [PATCH v6] tree-optimization/101186 - extend FRE with "equivalence
> map" for condition prediction
> 
> Sorry for the late update. I've been on a vacation and then I
> spent some time updating and verifying the patch.
> 
> Attached is a new version of the patch. There are some changes:
> 
> 1. Store equivalences in a vn_pval chain in vn_ssa_aux, rather than
>in the expression hash table. (Following Richard's suggestion.)
> 2. Extracted insert_single_predicated_value function.
> 3. Simplify record_equiv_from_prev_phi a bit.
> 4. Changed some of the functions' names and tried to improve the
>comments a little.
> 
> Current status of the new testcases in the patch:
> 
> ssa-fre-200.c Can also be optimized by evrp
> ssa-fre-201.c Not optimized in trunk.
> ssa-fre-202.c foo() can be removed by evrp; while x + b is not
>   folded.
> ssa-pre-34.c  Not optimized in trunk.
> 
> Initially, this patch is motivated to remove the unreachable codes
> in case like ssa-pre-34.c, in which we need to use equivalence
> relation produced from a preceding condition for another condition.
> VRP didn't optimize that because it needs jump threading to make
> the relation valid at the second condition.
> 
> After browsing the mechanisms of VRP and FRE, it seems to me there
> are two options: 1) Teach VRP to identify related but not threaded
> conditions. That might require introducing value-numbering into VRP
> to detect common expressions, and I think is too much for this.
> 2) Introduce temporary equivalence in sccvn, which I thought would
> change less on current code. (And along the reviews and updating
> patch I see how ad-hoc it was.)
> 
> I saw from the talk about VN there's plan to replace predicated
> values by ranger. So how does it goes? Is there something I can help
> with? (For the case ssa-pre-34.c, I think maybe it still needs the
> predicated-value support, to lookup related conditional expressions.)
> 
> Below are about questions in the last review:
> 
> > >  /* Valid hashtables storing information we have proven to be
> > > correct.  */
> > > @@ -490,9 +492,9 @@ VN_INFO (tree name)
> > >   nary->predicated_values = 0;
> > >   nary->u.result = boolean_true_node;
> > >   vn_nary_op_insert_into (nary, valid_info->nary);
> > > - gcc_assert (nary->unwind_to == NULL);
> >
> > why's that?  doesn't this mean unwinding will be broken?
> 
> Previously, predicate "argument_x == NULL" or "argument_x != NULL"
> is always new here (because argument_x's VN is just inserted.)
> But with the patch, there can be slot for "argument_x == NULL"
> or "argument_x != NULL" already. It won't break unwinding as the
> new value is not linked to the unwind-chain.
> 
> >
> > >   /* Also do not link it into the undo chain.  */
> > >   last_inserted_nary = nary->next;
> > > + /* There could be a predicate already.  */
> > >   nary->next = (vn_nary_op_t)(void *)-1;
> > >   nary = alloc_vn_nary_op_noinit (2, &vn_tables_insert_obstack);
> > >   init_vn_nary_op_from_pieces (nary, 2, EQ_EXPR,
> 
> > >  /* Compute and return the hash value for nary operation VBO1.  */
> > >
> > >  hashval_t
> > > @@ -4226,6 +4342,9 @@ init_vn_nary_op_from_stmt (vn_nary_op_t vno, gassign
> *stmt)
> > >for (i = 0; i < vno->length; ++i)
> > >   vno->op[i] = gimple_op (stmt, i + 1);
> > >  }
> > > +  /* Insert and lookup N-ary results by the operands' equivalence heads.
> */
> > > +  if (gimple_bb (stmt))
> > > +lookup_equiv_heads (vno->length, vno->op, vno->op, gimple_bb (stmt));
> >
> > That seems like the wrong place, the function didn't even valueize before.
> 
> To utilize temp-equivalences and get more simplified result, n-ary
> expressions should be always inserted and lookup by the operands'
> equivalence heads. So practically all the places
> init_vn_nary_op_from_stmt is used, lookup_equiv_heads (changed to
> get_equiv_heads) should be called. As I haven't found better place
> to put that, I just left it here in the patch..
> 
> > >  visit_nary_op (tree lhs, gassign *stmt)
> > >  {
> > >vn_nary_op_t vnresult;
> > > -  tree result = vn_nary_op_lookup_stmt (stmt, &vnresult);
> > > -  if (! result && vnresult)
> > > +  unsigned length = vn_nary_length_from_stmt (stmt);
> > > +  vn_nary_op_t vno
> > > += XALLOCAVAR (struct vn_nary_op_s, sizeof_vn_nary_op (length));
> > > +  init_vn_nary_op_from_stmt (vno, stmt);
> > > +  tree result = NULL_TREE;
> > > +  /* Try to get a simplified result.  */
> > > +  /* Do not simplify variable used in PHI at loop exit, or
> > > + simplify_peeled_chrec/constant_after_peeling may miss the loop.  */
> > > +  gimple *use_stmt;
>

Re: [PATCH] [PR68097] Try to avoid recursing for floats in tree_*_nonnegative_warnv_p.

[Aldy Hernandez via Gcc-patches]

On 11/15/22 08:15, Richard Biener wrote:

On Mon, Nov 14, 2022 at 8:05 PM Aldy Hernandez  wrote:




On 11/14/22 10:12, Richard Biener wrote:

On Sat, Nov 12, 2022 at 7:30 PM Aldy Hernandez  wrote:


It irks me that a PR named "we should track ranges for floating-point
hasn't been closed in this release.  This is an attempt to do just
that.

As mentioned in the PR, even though we track ranges for floats, it has
been suggested that avoiding recursing through SSA defs in
gimple_assign_nonnegative_warnv_p is also a goal.  We can do this with
various ranger components without the need for a heavy handed approach
(i.e. a full ranger).

I have implemented two versions of known_float_sign_p() that answer
the question whether we definitely know the sign for an operation or a
tree expression.

Both versions use get_global_range_query, which is a wrapper to query
global ranges.  This means, that no caching or propagation is done.
In the case of an SSA, we just return the global range for it (think
SSA_NAME_RANGE_INFO).  In the case of a tree code with operands, we
also use get_global_range_query to resolve the operands, and then call
into range-ops, which is our lowest level component.  There is no
ranger or gori involved.  All we're doing is resolving the operation
with the ranges passed.

This is enough to avoid recursing in the case where we definitely know
the sign of a range.  Otherwise, we still recurse.

Note that instead of get_global_range_query(), we could use
get_range_query() which uses a ranger (if active in a pass), or
get_global_range_query if not.  This would allow passes that have an
active ranger (with enable_ranger) to use a full ranger.  These passes
are currently, VRP, loop unswitching, DOM, loop versioning, etc.  If
no ranger is active, get_range_query defaults to global ranges, so
there's no additional penalty.

Would this be acceptable, at least enough to close (or rename the PR ;-))?


I think the checks would belong to the gimple_stmt_nonnegative_warnv_p function
only (that's the SSA name entry from the fold-const.cc ones)?


That was my first approach, but I thought I'd cover the unary and binary
operators as well, since they had other callers.  But I'm happy with
just the top-level tweak.  It's a lot less code :).


@@ -9234,6 +9235,15 @@ bool
  gimple_stmt_nonnegative_warnv_p (gimple *stmt, bool *strict_overflow_p,
  int depth)
  {
+  tree type = gimple_range_type (stmt);
+  if (type && frange::supports_p (type))
+{
+  frange r;
+  bool sign;
+  return (get_global_range_query ()->range_of_stmt (r, stmt)
+ && r.signbit_p (sign)
+ && sign == false);
+}

the above means we never fall through to the switch below if
frange::supports_p (type) - that's eventually good enough, I
don't think we ever call this very function directly but it gets
invoked via recursion through operands only.  But of course


Woah, sorry.  That was not intended.  For that matter, the patch as 
posted caused:


FAIL: gcc.dg/builtins-10.c (test for excess errors)
FAIL: gcc.dg/builtins-57.c (test for excess errors)
FAIL: gcc.dg/torture/builtin-nonneg-1.c   -O1  (test for excess errors)
FAIL: gcc.dg/torture/builtin-nonneg-1.c   -O2  (test for excess errors)
FAIL: gcc.dg/torture/builtin-nonneg-1.c   -O2 -flto 
-fno-use-linker-plugin -flto-partition=none  (test for excess errors)

FAIL: gcc.dg/torture/builtin-nonneg-1.c   -O3 -g  (test for excess errors)
FAIL: gcc.dg/torture/builtin-nonneg-1.c   -Os  (test for excess errors)
FAIL: gcc.dg/torture/builtin-power-1.c   -O1  (test for excess errors)
FAIL: gcc.dg/torture/builtin-power-1.c   -O2  (test for excess errors)
FAIL: gcc.dg/torture/builtin-power-1.c   -O2 -flto 
-fno-use-linker-plugin -flto-partition=none  (test for excess errors)

FAIL: gcc.dg/torture/builtin-power-1.c   -O3 -g  (test for excess errors)
FAIL: gcc.dg/torture/builtin-power-1.c   -Os  (test for excess errors)

Note that ranger folding calls this function, though it won't run the 
risk of endless recursion because range_of_stmt uses the LHS, and only 
use global ranges to solve the LHS.


Also, frange::supports_p() does not support all floats:

  static bool supports_p (const_tree type)
  {
// ?? Decimal floats can have multiple representations for the
// same number.  Supporting them may be as simple as just
// disabling them in singleton_p.  No clue.
return SCALAR_FLOAT_TYPE_P (type) && !DECIMAL_FLOAT_TYPE_P (type);
  }

Finally, my patch is more conservative than what the *nonnegative_warn* 
friends do.  We only return true when we're sure about the sign bit and 
it's FALSE.  As I mentioned elsewhere, tree_call_nonnegative_warn_p() 
always returns true for:


CASE_CFN_ACOS:
CASE_CFN_ACOS_FN:
CASE_CFN_ACOSH:
CASE_CFN_ACOSH_FN:
CASE_CFN_CABS:
CASE_CFN_CABS_FN:
...
...
  /* Always true.  */
  return true;

This means that we'll return true for a NAN, but we're incorrectly 
assuming 

Re: [PATCH 2/2]AArch64 Support new tbranch optab.

[Richard Sandiford via Gcc-patches]

Tamar Christina  writes:
>> -Original Message-
>> From: Richard Sandiford 
>> Sent: Tuesday, November 15, 2022 10:36 AM
>> To: Tamar Christina 
>> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
>> ; nd ; Marcus Shawcroft
>> 
>> Subject: Re: [PATCH 2/2]AArch64 Support new tbranch optab.
>> 
>> Tamar Christina  writes:
>> > Hello,
>> >
>> > Ping and updated patch.
>> >
>> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>> >
>> > Ok for master?
>> >
>> > Thanks,
>> > Tamar
>> >
>> > gcc/ChangeLog:
>> >
>> > * config/aarch64/aarch64.md (*tb1): Rename to...
>> > (*tb1): ... this.
>> > (tbranch4): New.
>> >
>> > gcc/testsuite/ChangeLog:
>> >
>> > * gcc.target/aarch64/tbz_1.c: New test.
>> >
>> > --- inline copy of patch ---
>> >
>> > diff --git a/gcc/config/aarch64/aarch64.md
>> > b/gcc/config/aarch64/aarch64.md index
>> >
>> 2bc2684b82c35a44e0a2cea6e3aaf32d939f8cdf..d7684c93fba5b717d568e1a4fd
>> 71
>> > 2bde55c7c72e 100644
>> > --- a/gcc/config/aarch64/aarch64.md
>> > +++ b/gcc/config/aarch64/aarch64.md
>> > @@ -943,12 +943,29 @@ (define_insn "*cb1"
>> >   (const_int 1)))]
>> >  )
>> >
>> > -(define_insn "*tb1"
>> > +(define_expand "tbranch4"
>> >[(set (pc) (if_then_else
>> > - (EQL (zero_extract:DI (match_operand:GPI 0 "register_operand"
>> "r")
>> > -   (const_int 1)
>> > -   (match_operand 1
>> > - "aarch64_simd_shift_imm_" "n"))
>> > +   (match_operator 0 "aarch64_comparison_operator"
>> > +[(match_operand:ALLI 1 "register_operand")
>> > + (match_operand:ALLI 2
>> "aarch64_simd_shift_imm_")])
>> > +   (label_ref (match_operand 3 "" ""))
>> > +   (pc)))]
>> > +  "optimize > 0"
>> 
>> Why's the pattern conditional on optimize?  Seems a valid choice at -O0 too.
>> 
>
> Hi,
>
> I had explained the reason why in the original patch, just didn't repeat it 
> in the ping:
>
> Instead of emitting the instruction directly I've chosen to expand the 
> pattern using a zero extract and generating the existing pattern for 
> comparisons for two
> reasons:
>
>   1. Allows for CSE of the actual comparison.
>   2. It looks like the code in expand makes the label as unused and removed it
>  if it doesn't see a separate reference to it.
>
> Because of this expansion though I disable the pattern at -O0 since we have 
> no combine in that case so we'd end up with worse code.  I did try emitting 
> the pattern directly, but as mentioned in no#2 expand would then kill the 
> label.
>
> Basically I emit the pattern directly, immediately during expand the label is 
> marked as dead for some weird reason.

Isn't #2 a bug though?  It seems like something we should fix rather than
work around.

Thanks,
Richard


>
> Tamar.
>
>> I think the split here shows the difficulty with having a single optab and a
>> comparison operator though.  operand 0 can be something like:
>> 
>>   (eq x 1)
>> 
>> but we're not comparing x for equality with 1.  We're testing whether bit 1 
>> is
>> zero.  This means that operand 0 can't be taken literally and can't be used
>> directly in insn patterns.
>> 
>> In an earlier review, I'd said:
>> 
>>   For the TB instructions (and for other similar instructions that I've
>>   seen on other architectures) it would be more useful to have a single-bit
>>   test, with operand 4 specifying the bit position.  Arguably it might then
>>   be better to have separate eq and ne optabs, to avoid the awkward
>> doubling
>>   of the operands (operand 1 contains operands 2 and 3).
>> 
>> I think we should do that eq/ne split (sorry for not pushing harder for it
>> before).
>> 
>> Thanks,
>> Richard
>> 
>> 
>> 
>> > +{
>> > +  rtx bitvalue = gen_reg_rtx (DImode);
>> > +  rtx tmp = simplify_gen_subreg (DImode, operands[1], GET_MODE
>> > +(operands[1]), 0);
>> > +  emit_insn (gen_extzv (bitvalue, tmp, const1_rtx, operands[2]));
>> > +  operands[2] = const0_rtx;
>> > +  operands[1] = aarch64_gen_compare_reg (GET_CODE (operands[0]),
>> bitvalue,
>> > +operands[2]);
>> > +})
>> > +
>> > +(define_insn "*tb1"
>> > +  [(set (pc) (if_then_else
>> > + (EQL (zero_extract:GPI (match_operand:ALLI 0 
>> > "register_operand"
>> "r")
>> > +(const_int 1)
>> > +(match_operand 1
>> > +
>> > +"aarch64_simd_shift_imm_" "n"))
>> >(const_int 0))
>> >  (label_ref (match_operand 2 "" ""))
>> >  (pc)))
>> > @@ -959,15 +976,15 @@ (define_insn "*tb1"
>> >{
>> > if (get_attr_far_branch (insn) == 1)
>> >   return aarch64_gen_far_branch (operands, 2, "Ltb",
>> > -"\\t%0, %1, ");
>> > +"\\t%0, %1,
>> > + ");
>> > else
>> >

Re: [PATCH] doc: Reword the description of -mrelax-cmpxchg-loop [PR 107676]

[Jonathan Wakely via Gcc-patches]

On 15/11/22 11:35 +0800, Hongyu Wang wrote:

Hi,

According to PR 107676, the document of -mrelax-cmpxchg-loop is nonsensical.
Adjust the wording according to the comments.

Bootstrapped on x86_64-pc-linux-gnu, ok for trunk?

gcc/ChangeLog:

PR target/107676
* doc/invoke.texi: Reword the description of
-mrelax-cmpxchg-loop.
---
gcc/doc/invoke.texi | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 40f667a630a..bdd7c319aef 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -33805,10 +33805,12 @@ registers.

@item -mrelax-cmpxchg-loop
@opindex mrelax-cmpxchg-loop
-Relax cmpxchg loop by emitting an early load and compare before cmpxchg,
-execute pause if load value is not expected. This reduces excessive
-cachline bouncing when and works for all atomic logic fetch builtins
-that generates compare and swap loop.
+For compare and swap loops that emitted by some __atomic_* builtins


s/that emitted/that are emitted/


+(e.g. __atomic_fetch_(or|and|xor|nand) and their __atomic_*_fetch
+counterparts), emit an atomic load before cmpxchg instruction. If the


s/before cmpxchg/before the cmpxchg/


+loaded value is not equal to expected, execute a pause instead of


s/not equal to expected/not equal to the expected/


+directly run the cmpxchg instruction. This might reduce excessive


s/directly run/directly running/


+cacheline bouncing.

@item -mindirect-branch=@var{choice}
@opindex mindirect-branch

RE: [PATCH 2/2]AArch64 Support new tbranch optab.

[Tamar Christina via Gcc-patches]

> -Original Message-
> From: Richard Sandiford 
> Sent: Tuesday, November 15, 2022 10:51 AM
> To: Tamar Christina 
> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
> ; nd ; Marcus Shawcroft
> 
> Subject: Re: [PATCH 2/2]AArch64 Support new tbranch optab.
> 
> Tamar Christina  writes:
> >> -Original Message-
> >> From: Richard Sandiford 
> >> Sent: Tuesday, November 15, 2022 10:36 AM
> >> To: Tamar Christina 
> >> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
> >> ; nd ; Marcus Shawcroft
> >> 
> >> Subject: Re: [PATCH 2/2]AArch64 Support new tbranch optab.
> >>
> >> Tamar Christina  writes:
> >> > Hello,
> >> >
> >> > Ping and updated patch.
> >> >
> >> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> >> >
> >> > Ok for master?
> >> >
> >> > Thanks,
> >> > Tamar
> >> >
> >> > gcc/ChangeLog:
> >> >
> >> > * config/aarch64/aarch64.md (*tb1): Rename to...
> >> > (*tb1): ... this.
> >> > (tbranch4): New.
> >> >
> >> > gcc/testsuite/ChangeLog:
> >> >
> >> > * gcc.target/aarch64/tbz_1.c: New test.
> >> >
> >> > --- inline copy of patch ---
> >> >
> >> > diff --git a/gcc/config/aarch64/aarch64.md
> >> > b/gcc/config/aarch64/aarch64.md index
> >> >
> >>
> 2bc2684b82c35a44e0a2cea6e3aaf32d939f8cdf..d7684c93fba5b717d568e1a4fd
> >> 71
> >> > 2bde55c7c72e 100644
> >> > --- a/gcc/config/aarch64/aarch64.md
> >> > +++ b/gcc/config/aarch64/aarch64.md
> >> > @@ -943,12 +943,29 @@ (define_insn "*cb1"
> >> >   (const_int 1)))]
> >> >  )
> >> >
> >> > -(define_insn "*tb1"
> >> > +(define_expand "tbranch4"
> >> >[(set (pc) (if_then_else
> >> > - (EQL (zero_extract:DI (match_operand:GPI 0 
> >> > "register_operand"
> >> "r")
> >> > -   (const_int 1)
> >> > -   (match_operand 1
> >> > - "aarch64_simd_shift_imm_" 
> >> > "n"))
> >> > +   (match_operator 0 "aarch64_comparison_operator"
> >> > +[(match_operand:ALLI 1 "register_operand")
> >> > + (match_operand:ALLI 2
> >> "aarch64_simd_shift_imm_")])
> >> > +   (label_ref (match_operand 3 "" ""))
> >> > +   (pc)))]
> >> > +  "optimize > 0"
> >>
> >> Why's the pattern conditional on optimize?  Seems a valid choice at -O0
> too.
> >>
> >
> > Hi,
> >
> > I had explained the reason why in the original patch, just didn't repeat it 
> > in
> the ping:
> >
> > Instead of emitting the instruction directly I've chosen to expand the
> > pattern using a zero extract and generating the existing pattern for
> > comparisons for two
> > reasons:
> >
> >   1. Allows for CSE of the actual comparison.
> >   2. It looks like the code in expand makes the label as unused and removed
> it
> >  if it doesn't see a separate reference to it.
> >
> > Because of this expansion though I disable the pattern at -O0 since we
> have no combine in that case so we'd end up with worse code.  I did try
> emitting the pattern directly, but as mentioned in no#2 expand would then
> kill the label.
> >
> > Basically I emit the pattern directly, immediately during expand the label 
> > is
> marked as dead for some weird reason.
> 
> Isn't #2 a bug though?  It seems like something we should fix rather than
> work around.

Yes it's a bug ☹ ok if I'm going to fix that bug then do I need to split the 
optabs
still? Isn't the problem atm that I need the split?  If I'm emitting the 
instruction
directly then the recog pattern for it can just be (eq (vec_extract x 1) 0) 
which is
the correct semantics?

Thanks,
Tamar
> 
> Thanks,
> Richard
> 
> 
> >
> > Tamar.
> >
> >> I think the split here shows the difficulty with having a single
> >> optab and a comparison operator though.  operand 0 can be something
> like:
> >>
> >>   (eq x 1)
> >>
> >> but we're not comparing x for equality with 1.  We're testing whether
> >> bit 1 is zero.  This means that operand 0 can't be taken literally
> >> and can't be used directly in insn patterns.
> >>
> >> In an earlier review, I'd said:
> >>
> >>   For the TB instructions (and for other similar instructions that I've
> >>   seen on other architectures) it would be more useful to have a single-bit
> >>   test, with operand 4 specifying the bit position.  Arguably it might then
> >>   be better to have separate eq and ne optabs, to avoid the awkward
> >> doubling
> >>   of the operands (operand 1 contains operands 2 and 3).
> >>
> >> I think we should do that eq/ne split (sorry for not pushing harder
> >> for it before).
> >>
> >> Thanks,
> >> Richard
> >>
> >>
> >>
> >> > +{
> >> > +  rtx bitvalue = gen_reg_rtx (DImode);
> >> > +  rtx tmp = simplify_gen_subreg (DImode, operands[1], GET_MODE
> >> > +(operands[1]), 0);
> >> > +  emit_insn (gen_extzv (bitvalue, tmp, const1_rtx, operands[2]));
> >> > +  operands[2] = const0_rtx;
> >> > +  operands[1] = aarch64_gen_compare_reg (GET_CODE (operands[0]),
> >> bitvalue,
> >> > + 

Re: [PATCH]AArch64 Extend umov and sbfx patterns.

[Richard Sandiford via Gcc-patches]

Tamar Christina  writes:
> Hi,
>
>> > --- a/gcc/config/aarch64/aarch64-simd.md
>> > +++ b/gcc/config/aarch64/aarch64-simd.md
>> > @@ -4259,7 +4259,7 @@ (define_insn
>> "*aarch64_get_lane_zero_extend"
>> >  ;; Extracting lane zero is split into a simple move when it is
>> > between SIMD  ;; registers or a store.
>> >  (define_insn_and_split "aarch64_get_lane"
>> > -  [(set (match_operand: 0 "aarch64_simd_nonimmediate_operand"
>> > "=?r, w, Utv")
>> > +  [(set (match_operand: 0 "aarch64_simd_nonimmediate_operand"
>> > + "=r, w, Utv")
>> >(vec_select:
>> >  (match_operand:VALL_F16_FULL 1 "register_operand" "w, w, w")
>> >  (parallel [(match_operand:SI 2 "immediate_operand" "i, i, i")])))]
>> 
>> Which testcase does this help with?  It didn't look like the new tests do any
>> vector stuff.
>> 
>
> Right, sorry about that, splitting up my patches resulted in this sneaking in 
> from a different series.
> Moved now.
>
>> > -(define_insn "*_ashl"
>> > +(define_insn "*_ashl"
>> >[(set (match_operand:GPI 0 "register_operand" "=r")
>> >(ANY_EXTEND:GPI
>> > -   (ashift:SHORT (match_operand:SHORT 1 "register_operand" "r")
>> > +   (ashift:ALLX (match_operand:ALLX 1 "register_operand" "r")
>> >   (match_operand 2 "const_int_operand" "n"]
>> > -  "UINTVAL (operands[2]) < GET_MODE_BITSIZE (mode)"
>> > +  "UINTVAL (operands[2]) < GET_MODE_BITSIZE (mode)"
>> 
>> It'd be better to avoid even defining si<-si or si<-di "extensions"
>> (even though nothing should try to match them), so how about adding:
>> 
>>>  &&
>> 
>> or similar to the beginning of the condition?  The conditions for the invalid
>> combos will then be provably false at compile time and the patterns will be
>> compiled out.
>> 
>
> Done.
>
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>
> Ok for master?
>
> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
>   * config/aarch64/aarch64.md
>   (*_ashl): Renamed to...
>   (*_ashl): ...this.
>   (*zero_extend_lshr): Renamed to...
>   (*zero_extend_lshr): ...this.
>   (*extend_ashr): Rename to...
>   (*extend_ashr): ...this.
>
> gcc/testsuite/ChangeLog:
>
>   * gcc.target/aarch64/bitmove_1.c: New test.
>   * gcc.target/aarch64/bitmove_2.c: New test.
>
> --- inline copy of patch ---
>
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 
> d7684c93fba5b717d568e1a4fd712bde55c7c72e..d230bbb833f97813c8371aa07b587bd8b0292cee
>  100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -5711,40 +5711,43 @@ (define_insn "*extrsi5_insn_di"
>[(set_attr "type" "rotate_imm")]
>  )
>  
> -(define_insn "*_ashl"
> +(define_insn "*_ashl"
>[(set (match_operand:GPI 0 "register_operand" "=r")
>   (ANY_EXTEND:GPI
> -  (ashift:SHORT (match_operand:SHORT 1 "register_operand" "r")
> +  (ashift:ALLX (match_operand:ALLX 1 "register_operand" "r")
>  (match_operand 2 "const_int_operand" "n"]
> -  "UINTVAL (operands[2]) < GET_MODE_BITSIZE (mode)"
> +  " > 
> +   && UINTVAL (operands[2]) < GET_MODE_BITSIZE (mode)"
>  {
> -  operands[3] = GEN_INT ( - UINTVAL (operands[2]));
> +  operands[3] = GEN_INT ( - UINTVAL (operands[2]));
>return "bfiz\t%0, %1, %2, %3";
>  }
>[(set_attr "type" "bfx")]
>  )
>  
> -(define_insn "*zero_extend_lshr"
> +(define_insn "*zero_extend_lshr"
>[(set (match_operand:GPI 0 "register_operand" "=r")
>   (zero_extend:GPI
> -  (lshiftrt:SHORT (match_operand:SHORT 1 "register_operand" "r")
> -  (match_operand 2 "const_int_operand" "n"]
> -  "UINTVAL (operands[2]) < GET_MODE_BITSIZE (mode)"
> +  (lshiftrt:ALLX (match_operand:ALLX 1 "register_operand" "r")
> + (match_operand 2 "const_int_operand" "n"]
> +  " > 
> +   && UINTVAL (operands[2]) < GET_MODE_BITSIZE (mode)"
>  {
> -  operands[3] = GEN_INT ( - UINTVAL (operands[2]));
> +  operands[3] = GEN_INT ( - UINTVAL (operands[2]));
>return "ubfx\t%0, %1, %2, %3";
>  }
>[(set_attr "type" "bfx")]
>  )
>  
> -(define_insn "*extend_ashr"
> +(define_insn "*extend_ashr"
>[(set (match_operand:GPI 0 "register_operand" "=r")
>   (sign_extend:GPI
> -  (ashiftrt:SHORT (match_operand:SHORT 1 "register_operand" "r")
> -  (match_operand 2 "const_int_operand" "n"]
> -  "UINTVAL (operands[2]) < GET_MODE_BITSIZE (mode)"
> +  (ashiftrt:ALLX (match_operand:ALLX 1 "register_operand" "r")
> + (match_operand 2 "const_int_operand" "n"]
> +  " > 
> +   && UINTVAL (operands[2]) < GET_MODE_BITSIZE (mode)"
>  {
> -  operands[3] = GEN_INT ( - UINTVAL (operands[2]));
> +  operands[3] = GEN_INT ( - UINTVAL (operands[2]));
>return "sbfx\\t%0, %1, %2, %3";
>  }
>[(set_attr "type" "bfx")]
> diff --git a/gcc/testsuite/gcc.target/aarch64/bitmove_1.c 
> b/gcc/testsuite/gcc.target/aarch64/bitmove_1.c
> new file mode 100644
> index 
> 000

Re: [PATCH 2/2]AArch64 Support new tbranch optab.

[Richard Sandiford via Gcc-patches]

Tamar Christina  writes:
>> -Original Message-
>> From: Richard Sandiford 
>> Sent: Tuesday, November 15, 2022 10:51 AM
>> To: Tamar Christina 
>> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
>> ; nd ; Marcus Shawcroft
>> 
>> Subject: Re: [PATCH 2/2]AArch64 Support new tbranch optab.
>> 
>> Tamar Christina  writes:
>> >> -Original Message-
>> >> From: Richard Sandiford 
>> >> Sent: Tuesday, November 15, 2022 10:36 AM
>> >> To: Tamar Christina 
>> >> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
>> >> ; nd ; Marcus Shawcroft
>> >> 
>> >> Subject: Re: [PATCH 2/2]AArch64 Support new tbranch optab.
>> >>
>> >> Tamar Christina  writes:
>> >> > Hello,
>> >> >
>> >> > Ping and updated patch.
>> >> >
>> >> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>> >> >
>> >> > Ok for master?
>> >> >
>> >> > Thanks,
>> >> > Tamar
>> >> >
>> >> > gcc/ChangeLog:
>> >> >
>> >> > * config/aarch64/aarch64.md (*tb1): Rename to...
>> >> > (*tb1): ... this.
>> >> > (tbranch4): New.
>> >> >
>> >> > gcc/testsuite/ChangeLog:
>> >> >
>> >> > * gcc.target/aarch64/tbz_1.c: New test.
>> >> >
>> >> > --- inline copy of patch ---
>> >> >
>> >> > diff --git a/gcc/config/aarch64/aarch64.md
>> >> > b/gcc/config/aarch64/aarch64.md index
>> >> >
>> >>
>> 2bc2684b82c35a44e0a2cea6e3aaf32d939f8cdf..d7684c93fba5b717d568e1a4fd
>> >> 71
>> >> > 2bde55c7c72e 100644
>> >> > --- a/gcc/config/aarch64/aarch64.md
>> >> > +++ b/gcc/config/aarch64/aarch64.md
>> >> > @@ -943,12 +943,29 @@ (define_insn "*cb1"
>> >> >   (const_int 1)))]
>> >> >  )
>> >> >
>> >> > -(define_insn "*tb1"
>> >> > +(define_expand "tbranch4"
>> >> >[(set (pc) (if_then_else
>> >> > - (EQL (zero_extract:DI (match_operand:GPI 0 
>> >> > "register_operand"
>> >> "r")
>> >> > -   (const_int 1)
>> >> > -   (match_operand 1
>> >> > - "aarch64_simd_shift_imm_" 
>> >> > "n"))
>> >> > +   (match_operator 0 "aarch64_comparison_operator"
>> >> > +[(match_operand:ALLI 1 "register_operand")
>> >> > + (match_operand:ALLI 2
>> >> "aarch64_simd_shift_imm_")])
>> >> > +   (label_ref (match_operand 3 "" ""))
>> >> > +   (pc)))]
>> >> > +  "optimize > 0"
>> >>
>> >> Why's the pattern conditional on optimize?  Seems a valid choice at -O0
>> too.
>> >>
>> >
>> > Hi,
>> >
>> > I had explained the reason why in the original patch, just didn't repeat 
>> > it in
>> the ping:
>> >
>> > Instead of emitting the instruction directly I've chosen to expand the
>> > pattern using a zero extract and generating the existing pattern for
>> > comparisons for two
>> > reasons:
>> >
>> >   1. Allows for CSE of the actual comparison.
>> >   2. It looks like the code in expand makes the label as unused and removed
>> it
>> >  if it doesn't see a separate reference to it.
>> >
>> > Because of this expansion though I disable the pattern at -O0 since we
>> have no combine in that case so we'd end up with worse code.  I did try
>> emitting the pattern directly, but as mentioned in no#2 expand would then
>> kill the label.
>> >
>> > Basically I emit the pattern directly, immediately during expand the label 
>> > is
>> marked as dead for some weird reason.
>> 
>> Isn't #2 a bug though?  It seems like something we should fix rather than
>> work around.
>
> Yes it's a bug ☹ ok if I'm going to fix that bug then do I need to split the 
> optabs
> still? Isn't the problem atm that I need the split?  If I'm emitting the 
> instruction
> directly then the recog pattern for it can just be (eq (vec_extract x 1) 0) 
> which is
> the correct semantics?

What rtx does the code that uses the optab pass for operand 0?

Richard

Re: [PATCH][i386]: Update ix86_can_change_mode_class target hook to accept QImode conversions

[Hongtao Liu via Gcc-patches]

On Fri, Nov 11, 2022 at 10:47 PM Tamar Christina via Gcc-patches
 wrote:
>
> Hi All,
>
> The current i386 implementation of the TARGET_CAN_CHANGE_MODE_CLASS is 
> currently
> not useful before re-alloc.
>
> In particular before regalloc optimization passes query the hook using 
> ALL_REGS,
> but because of the
>
>   if (MAYBE_FLOAT_CLASS_P (regclass))
>   return false;
>
> The hook returns false for all modes, even integer ones because ALL_REGS
> overlaps with floating point regs.
>
> The vector permute fallback cases used to unconditionally convert vector 
> integer
> permutes to vector QImode ones as a fallback plan.  This is incorrect and can
> result in incorrect code if the target doesn't support this conversion.
>
> To fix this some more checks were added, however that ended up introducing 
> ICEs
> in the i386 backend because e.g. the hook would reject conversions between 
> modes
> like V2TImode and V32QImode.
>
> My understanding is that for x87 we don't want to allow floating point
> conversions, but integers are fine.  So I have modified the check such that it
> also checks the modes, not just the register class groups.
>
> The second part of the code is needed because now that integer modes aren't
> uniformly rejected the i386 backend trigger further optimizations.  However 
> the
> backend lacks instructions to deal with canonical RTL representations of
> certain instructions.  for instance the back-end seems to prefer vec_select 0
> instead of subregs.
If the canonical form of the lower half of the vector in rtl is subreg
instead of vec_select, the x86 backend needs to update the pattern
accordingly to avoid diverge.
I can reproduce those failures when adding VALID_FP_MODE_P in
targetm.can_change_mode_class.
>
> So to prevent the canonicalization I reject integer modes when the sizes of to
> and from don't match and when we would have exited with false previously.
>
> This fixes all the ICEs and codegen regressions, but perhaps an x86 maintainer
> should take a deeper look at this hook implementation.
>
> Bootstrapped Regtested on x86_64-pc-linux-gnu and no issues.
>
> Ok for master?
>
> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
> * config/i386/i386.cc (ix86_can_change_mode_class): Update the target
> hook.
>
> --- inline copy of patch --
> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> index 
> c4d0e36e9c0a2256f5dde1f4dc021c0328aa0cba..477dd007ea80272680751b61e35cc3eec79b66c3
>  100644
> --- a/gcc/config/i386/i386.cc
> +++ b/gcc/config/i386/i386.cc
> @@ -19682,7 +19682,15 @@ ix86_can_change_mode_class (machine_mode from, 
> machine_mode to,
>
>/* x87 registers can't do subreg at all, as all values are reformatted
>   to extended precision.  */
> -  if (MAYBE_FLOAT_CLASS_P (regclass))
> +  if (MAYBE_FLOAT_CLASS_P (regclass)
> +  && VALID_FP_MODE_P (from)
> +  && VALID_FP_MODE_P (to))
> +return false;
> +
> +  /* Reject integer modes if the sizes aren't the same.  It would have
> + normally exited above.  */
> +  if (MAYBE_FLOAT_CLASS_P (regclass)
> +  && GET_MODE_SIZE (from) != GET_MODE_SIZE (to))
>  return false;
>
>if (MAYBE_SSE_CLASS_P (regclass) || MAYBE_MMX_CLASS_P (regclass))
>
>
>
>
> --



-- 
BR,
Hongtao

RE: [PATCH 2/2]AArch64 Support new tbranch optab.

[Tamar Christina via Gcc-patches]

> -Original Message-
> From: Richard Sandiford 
> Sent: Tuesday, November 15, 2022 11:15 AM
> To: Tamar Christina 
> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
> ; nd ; Marcus Shawcroft
> 
> Subject: Re: [PATCH 2/2]AArch64 Support new tbranch optab.
> 
> Tamar Christina  writes:
> >> -Original Message-
> >> From: Richard Sandiford 
> >> Sent: Tuesday, November 15, 2022 10:51 AM
> >> To: Tamar Christina 
> >> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
> >> ; nd ; Marcus Shawcroft
> >> 
> >> Subject: Re: [PATCH 2/2]AArch64 Support new tbranch optab.
> >>
> >> Tamar Christina  writes:
> >> >> -Original Message-
> >> >> From: Richard Sandiford 
> >> >> Sent: Tuesday, November 15, 2022 10:36 AM
> >> >> To: Tamar Christina 
> >> >> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
> >> >> ; nd ; Marcus
> Shawcroft
> >> >> 
> >> >> Subject: Re: [PATCH 2/2]AArch64 Support new tbranch optab.
> >> >>
> >> >> Tamar Christina  writes:
> >> >> > Hello,
> >> >> >
> >> >> > Ping and updated patch.
> >> >> >
> >> >> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> >> >> >
> >> >> > Ok for master?
> >> >> >
> >> >> > Thanks,
> >> >> > Tamar
> >> >> >
> >> >> > gcc/ChangeLog:
> >> >> >
> >> >> > * config/aarch64/aarch64.md (*tb1): Rename
> to...
> >> >> > (*tb1): ... this.
> >> >> > (tbranch4): New.
> >> >> >
> >> >> > gcc/testsuite/ChangeLog:
> >> >> >
> >> >> > * gcc.target/aarch64/tbz_1.c: New test.
> >> >> >
> >> >> > --- inline copy of patch ---
> >> >> >
> >> >> > diff --git a/gcc/config/aarch64/aarch64.md
> >> >> > b/gcc/config/aarch64/aarch64.md index
> >> >> >
> >> >>
> >>
> 2bc2684b82c35a44e0a2cea6e3aaf32d939f8cdf..d7684c93fba5b717d568e1a4fd
> >> >> 71
> >> >> > 2bde55c7c72e 100644
> >> >> > --- a/gcc/config/aarch64/aarch64.md
> >> >> > +++ b/gcc/config/aarch64/aarch64.md
> >> >> > @@ -943,12 +943,29 @@ (define_insn "*cb1"
> >> >> >   (const_int 1)))]
> >> >> >  )
> >> >> >
> >> >> > -(define_insn "*tb1"
> >> >> > +(define_expand "tbranch4"
> >> >> >[(set (pc) (if_then_else
> >> >> > - (EQL (zero_extract:DI (match_operand:GPI 0
> "register_operand"
> >> >> "r")
> >> >> > -   (const_int 1)
> >> >> > -   (match_operand 1
> >> >> > - "aarch64_simd_shift_imm_" 
> >> >> > "n"))
> >> >> > +   (match_operator 0 "aarch64_comparison_operator"
> >> >> > +[(match_operand:ALLI 1 "register_operand")
> >> >> > + (match_operand:ALLI 2
> >> >> "aarch64_simd_shift_imm_")])
> >> >> > +   (label_ref (match_operand 3 "" ""))
> >> >> > +   (pc)))]
> >> >> > +  "optimize > 0"
> >> >>
> >> >> Why's the pattern conditional on optimize?  Seems a valid choice
> >> >> at -O0
> >> too.
> >> >>
> >> >
> >> > Hi,
> >> >
> >> > I had explained the reason why in the original patch, just didn't
> >> > repeat it in
> >> the ping:
> >> >
> >> > Instead of emitting the instruction directly I've chosen to expand
> >> > the pattern using a zero extract and generating the existing
> >> > pattern for comparisons for two
> >> > reasons:
> >> >
> >> >   1. Allows for CSE of the actual comparison.
> >> >   2. It looks like the code in expand makes the label as unused and
> >> > removed
> >> it
> >> >  if it doesn't see a separate reference to it.
> >> >
> >> > Because of this expansion though I disable the pattern at -O0 since
> >> > we
> >> have no combine in that case so we'd end up with worse code.  I did
> >> try emitting the pattern directly, but as mentioned in no#2 expand
> >> would then kill the label.
> >> >
> >> > Basically I emit the pattern directly, immediately during expand
> >> > the label is
> >> marked as dead for some weird reason.
> >>
> >> Isn't #2 a bug though?  It seems like something we should fix rather
> >> than work around.
> >
> > Yes it's a bug ☹ ok if I'm going to fix that bug then do I need to
> > split the optabs still? Isn't the problem atm that I need the split?
> > If I'm emitting the instruction directly then the recog pattern for it
> > can just be (eq (vec_extract x 1) 0) which is the correct semantics?
> 
> What rtx does the code that uses the optab pass for operand 0?

It gets passed the full comparison:

(eq (reg/v:SI 92 [ x ])
(const_int 0 [0]))

of which we only look at the operator.

Tamar.

> 
> Richard

Re: [PATCH] ira: Remove duplicate `memset' over `full_costs' from `assign_hard_reg'

[Maciej W. Rozycki]

On Mon, 14 Nov 2022, Jeff Law wrote:

> > gcc/
> > * ira-color.cc (assign_hard_reg): Remove duplicate `memset' over
> > `full_costs'.
> > ---
> > Hi,
> > 
> >   I find this fairly obvious, OK to apply?
> 
> Seems obvious to me as well.  OK.

 Thanks, now committed.

  Maciej

Re: [PATCH 2/2]AArch64 Support new tbranch optab.

[Richard Sandiford via Gcc-patches]

Tamar Christina  writes:
>> -Original Message-
>> From: Richard Sandiford 
>> Sent: Tuesday, November 15, 2022 11:15 AM
>> To: Tamar Christina 
>> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
>> ; nd ; Marcus Shawcroft
>> 
>> Subject: Re: [PATCH 2/2]AArch64 Support new tbranch optab.
>> 
>> Tamar Christina  writes:
>> >> -Original Message-
>> >> From: Richard Sandiford 
>> >> Sent: Tuesday, November 15, 2022 10:51 AM
>> >> To: Tamar Christina 
>> >> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
>> >> ; nd ; Marcus Shawcroft
>> >> 
>> >> Subject: Re: [PATCH 2/2]AArch64 Support new tbranch optab.
>> >>
>> >> Tamar Christina  writes:
>> >> >> -Original Message-
>> >> >> From: Richard Sandiford 
>> >> >> Sent: Tuesday, November 15, 2022 10:36 AM
>> >> >> To: Tamar Christina 
>> >> >> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
>> >> >> ; nd ; Marcus
>> Shawcroft
>> >> >> 
>> >> >> Subject: Re: [PATCH 2/2]AArch64 Support new tbranch optab.
>> >> >>
>> >> >> Tamar Christina  writes:
>> >> >> > Hello,
>> >> >> >
>> >> >> > Ping and updated patch.
>> >> >> >
>> >> >> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>> >> >> >
>> >> >> > Ok for master?
>> >> >> >
>> >> >> > Thanks,
>> >> >> > Tamar
>> >> >> >
>> >> >> > gcc/ChangeLog:
>> >> >> >
>> >> >> > * config/aarch64/aarch64.md (*tb1): Rename
>> to...
>> >> >> > (*tb1): ... this.
>> >> >> > (tbranch4): New.
>> >> >> >
>> >> >> > gcc/testsuite/ChangeLog:
>> >> >> >
>> >> >> > * gcc.target/aarch64/tbz_1.c: New test.
>> >> >> >
>> >> >> > --- inline copy of patch ---
>> >> >> >
>> >> >> > diff --git a/gcc/config/aarch64/aarch64.md
>> >> >> > b/gcc/config/aarch64/aarch64.md index
>> >> >> >
>> >> >>
>> >>
>> 2bc2684b82c35a44e0a2cea6e3aaf32d939f8cdf..d7684c93fba5b717d568e1a4fd
>> >> >> 71
>> >> >> > 2bde55c7c72e 100644
>> >> >> > --- a/gcc/config/aarch64/aarch64.md
>> >> >> > +++ b/gcc/config/aarch64/aarch64.md
>> >> >> > @@ -943,12 +943,29 @@ (define_insn "*cb1"
>> >> >> >   (const_int 1)))]
>> >> >> >  )
>> >> >> >
>> >> >> > -(define_insn "*tb1"
>> >> >> > +(define_expand "tbranch4"
>> >> >> >[(set (pc) (if_then_else
>> >> >> > - (EQL (zero_extract:DI (match_operand:GPI 0
>> "register_operand"
>> >> >> "r")
>> >> >> > -   (const_int 1)
>> >> >> > -   (match_operand 1
>> >> >> > - 
>> >> >> > "aarch64_simd_shift_imm_" "n"))
>> >> >> > +   (match_operator 0 "aarch64_comparison_operator"
>> >> >> > +[(match_operand:ALLI 1 "register_operand")
>> >> >> > + (match_operand:ALLI 2
>> >> >> "aarch64_simd_shift_imm_")])
>> >> >> > +   (label_ref (match_operand 3 "" ""))
>> >> >> > +   (pc)))]
>> >> >> > +  "optimize > 0"
>> >> >>
>> >> >> Why's the pattern conditional on optimize?  Seems a valid choice
>> >> >> at -O0
>> >> too.
>> >> >>
>> >> >
>> >> > Hi,
>> >> >
>> >> > I had explained the reason why in the original patch, just didn't
>> >> > repeat it in
>> >> the ping:
>> >> >
>> >> > Instead of emitting the instruction directly I've chosen to expand
>> >> > the pattern using a zero extract and generating the existing
>> >> > pattern for comparisons for two
>> >> > reasons:
>> >> >
>> >> >   1. Allows for CSE of the actual comparison.
>> >> >   2. It looks like the code in expand makes the label as unused and
>> >> > removed
>> >> it
>> >> >  if it doesn't see a separate reference to it.
>> >> >
>> >> > Because of this expansion though I disable the pattern at -O0 since
>> >> > we
>> >> have no combine in that case so we'd end up with worse code.  I did
>> >> try emitting the pattern directly, but as mentioned in no#2 expand
>> >> would then kill the label.
>> >> >
>> >> > Basically I emit the pattern directly, immediately during expand
>> >> > the label is
>> >> marked as dead for some weird reason.
>> >>
>> >> Isn't #2 a bug though?  It seems like something we should fix rather
>> >> than work around.
>> >
>> > Yes it's a bug ☹ ok if I'm going to fix that bug then do I need to
>> > split the optabs still? Isn't the problem atm that I need the split?
>> > If I'm emitting the instruction directly then the recog pattern for it
>> > can just be (eq (vec_extract x 1) 0) which is the correct semantics?
>> 
>> What rtx does the code that uses the optab pass for operand 0?
>
> It gets passed the full comparison:
>
> (eq (reg/v:SI 92 [ x ])
> (const_int 0 [0]))
>
> of which we only look at the operator.

OK, that's what I thought.  The problem is then the one I mentioned above.
This rtx doesn't describe the operation that the optab is supposed to
perform, so it can never be used in the instruction pattern.  (This is
different from something like cbranch, where operand 0 can be used directly
if the target supports a very general compare-and-branch instruction.)

If we want to use a single op

[committed] libstdc++: Document use of Markdown for Doxygen comments

[Jonathan Wakely via Gcc-patches]

Pushed to trunk.

-- >8 --

libstdc++-v3/ChangeLog:

* doc/xml/manual/documentation_hacking.xml: Document use of
Markdown for Doxygen comments. Tweak formatting.
* doc/html/manual/documentation_hacking.html: Regenerate.
---
 .../html/manual/documentation_hacking.html| 21 --
 .../doc/xml/manual/documentation_hacking.xml  | 28 ++-
 2 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/libstdc++-v3/doc/xml/manual/documentation_hacking.xml 
b/libstdc++-v3/doc/xml/manual/documentation_hacking.xml
index 776d5e857b5..44672f6e26d 100644
--- a/libstdc++-v3/doc/xml/manual/documentation_hacking.xml
+++ b/libstdc++-v3/doc/xml/manual/documentation_hacking.xml
@@ -286,7 +286,9 @@
formatting system, and will require the expansion of TeX's memory
capacity. Specifically, the pool_size
variable in the configuration file texmf.cnf may
-   need to be increased by a minimum factor of two.
+   need to be increased by a minimum factor of two. Alternatively, using
+   LATEX_CMD=lualatex might allow the docs to be
+   build without running out of memory.
   
 
 
@@ -515,9 +517,12 @@
   
 
   
-   Please use markup tags like @p and @a when referring to things
-   such as the names of function parameters. Use @e for emphasis
-   when necessary. Use @c to refer to other standard names.
+   Markdown can be used for formatting text. Doxygen is configured to
+   support this, and it is a good compromise between readable comments
+   in the C++ source and nice formatting in the generated HTML.
+   Please format the names of function parameters in either code font
+   or italics. Use underscores or @e for emphasis when necessary.
+   Use backticks or @c to refer to other standard names.
(Examples of all these abound in the present code.)
   
 
@@ -595,6 +600,7 @@
 
   HTML
   Doxygen
+  Markdown
 
   
 
@@ -602,41 +608,49 @@
 
   \
   \\
+  \\
 
 
 
   "
   \"
+  \"
 
 
 
   '
   \'
+  \'
 
 
 
   
   @a word
+  _word_ or *word*
 
 
 
   
   @b word
+  **word** or __word__
 
 
 
   
   @c word
+  `word`
 
 
 
   
   @a word
+  _word_ or *word*
 
 
 
   
   two words or more
+  _two words or more_
 
   
 
@@ -719,7 +733,7 @@
 
 
   
-   Editing the DocBook sources requires an XML editor. Many
+   An XML editor is recommended for editing the DocBook sources. Many
exist: some notable options
include emacs, Kate,
or Conglomerate.
@@ -815,8 +829,8 @@
   
 
   
-   The doc-html-docbook-regenerate target will generate
-   the HTML files and copy them back to the libstdc++ source tree.
+   The doc-html-docbook-regenerate target will
+   generate the HTML files and copy them back to the libstdc++ source tree.
This can be used to update the HTML files that are checked in to
version control.
   
-- 
2.38.1