[PATCH] RISC-V: Fix interrupt support for -g.

[Jim Wilson]

This fixes a problem found by someone trying to use the new RISC-V interrupt
attribute support.  With a slightly non-trivial example, and the -g option, we
get an abort in dwarf2cfi for an inconsistent CFI state.  This is my fault
for not making the new interrupt return patterns look enough like regular
return patterns, which is simple to fix.

Tested with cross riscv32-elf and riscv64-linux builds and tests.  There were
no regressions.  The new testcase fails without the patch and works with the
patch.

Committed.

Jim

gcc/
* config/riscv/riscv.c (riscv_expand_epilogue): Use emit_jump_insn
instead of emit_insn for interrupt returns.
* config/riscv/riscv.md (riscv_met): Add (return) to rtl.
(riscv_sret, riscv_uret): Likewise.

gcc/testsuite/
* gcc.target/riscv/interrupt-debug.c: New.
---
 gcc/config/riscv/riscv.c |  6 +++---
 gcc/config/riscv/riscv.md|  9 ++---
 gcc/testsuite/gcc.target/riscv/interrupt-debug.c | 15 +++
 3 files changed, 24 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/interrupt-debug.c

diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index 2709ebdd797..d87836f53f8 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -3985,11 +3985,11 @@ riscv_expand_epilogue (int style)
   enum riscv_privilege_levels mode = cfun->machine->interrupt_mode;
 
   if (mode == MACHINE_MODE)
-   emit_insn (gen_riscv_mret ());
+   emit_jump_insn (gen_riscv_mret ());
   else if (mode == SUPERVISOR_MODE)
-   emit_insn (gen_riscv_sret ());
+   emit_jump_insn (gen_riscv_sret ());
   else
-   emit_insn (gen_riscv_uret ());
+   emit_jump_insn (gen_riscv_uret ());
 }
   else if (style != SIBCALL_RETURN)
 emit_jump_insn (gen_simple_return_internal (ra));
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 7b411f0538e..613af9d79e4 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -2328,17 +2328,20 @@
   "fsflags\t%0")
 
 (define_insn "riscv_mret"
-  [(unspec_volatile [(const_int 0)] UNSPECV_MRET)]
+  [(return)
+   (unspec_volatile [(const_int 0)] UNSPECV_MRET)]
   ""
   "mret")
 
 (define_insn "riscv_sret"
-  [(unspec_volatile [(const_int 0)] UNSPECV_SRET)]
+  [(return)
+   (unspec_volatile [(const_int 0)] UNSPECV_SRET)]
   ""
   "sret")
 
 (define_insn "riscv_uret"
-  [(unspec_volatile [(const_int 0)] UNSPECV_URET)]
+  [(return)
+   (unspec_volatile [(const_int 0)] UNSPECV_URET)]
   ""
   "uret")
 
diff --git a/gcc/testsuite/gcc.target/riscv/interrupt-debug.c 
b/gcc/testsuite/gcc.target/riscv/interrupt-debug.c
new file mode 100644
index 000..a1b6dac8fbb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/interrupt-debug.c
@@ -0,0 +1,15 @@
+/* Verify that we can compile with debug info.  */
+/* { dg-do compile } */
+/* { dg-options "-Og -g" } */
+extern int var1;
+extern int var2;
+extern void sub2 (void);
+
+void __attribute__ ((interrupt))
+sub (void)
+{
+  if (var1)
+var2 = 0;
+  else
+sub2 ();
+}
-- 
2.17.1

Re: [PATCH] RISC-V: Fix interrupt support for -g.

[Jim Wilson]

On Mon, Jul 2, 2018 at 8:04 PM, Kito Cheng  wrote:
> Does it possible just combine those pattern into simple_return
> pattern, and then check the function type and output correct return
> instruction in riscv_output_return?

There might be problems with optimizations thinking this is a regular
return when it isn't.  But it might work if we have enough checks for
the interrupt attribute in the right places.

Having different patterns for different types of return instructions
makes the RTL dumps self contained.  You can tell what kind of
instruction it is without having to look at the source tree to see how
the function was declared.  There is some minor benefit to that.

Is there a problem with the current approach that needs fixing?

Jim

Re: [PATCH] Update config.guess and config.sub

[Jim Wilson]

On 07/05/2018 09:51 AM, Palmer Dabbelt wrote:
When I try to build it I see "Unsupported RISC-V target 
riscv-unknown-elf", so there's at least some extra autoconf wizadry that 
needs to happen in here.  I'm actually not sure what the "riscv-*" 
tuples are supposed to do so I've added Liviu as I don't want to 
misrepresent his desires and get into trouble again :).


I objected to this on the config.sub package mailing list, because it 
adds riscv-linux as a valid configure tuple, and we never wanted that. 
But this seems to be a losing battle.


Jim

Re: [PATCH] Update config.guess and config.sub

[Jim Wilson]

On Thu, Jul 5, 2018 at 11:31 AM, Liviu Ionescu  wrote:
> If this is really a problem I guess you can blacklist it somehow.
> But this proves once again that Linux native compilers and cross embedded 
> toolchains should be processed differently.

It isn't a major problem.  There is the issue that the more different
configure triplets we have the more work I need to do to keep them all
working.  So from my viewpoint, I'd rather not have riscv-*, but now
that we have it, I can fix the gcc config.sub to make it work.  On the
linux side, the expectation is that everyone will be using
riscv64-linux, and we don't have any usable upstream riscv32-linux
support yet, so there isn't any problem (yet).

Jim

[PATCH, Ada] RISC-V: Initial riscv linux Ada port.

[Jim Wilson]

I was asked about Ada support, so I tried cross building a native RISC-V Linux
Ada compiler, and it turned out to be possible with a little bit of work.  I
just started with the MIPS support, and then fixed everything that was
obviously wrong: endianness, error numbers, signal numbers, struct_sigaction
offsets, etc.

The result is good enough to bootstrap natively and seems to give reasonable
native testsuite results for a first attempt.  The machine I'm running on has
broken icache flushing, so trampolines won't work, and I suspect that is
causing a lot of the testsuite failures.  Here are the Ada testsuite results
I'm getting at the moment.

=== acats Summary ===
# of expected passes2138
# of unexpected failures182

=== gnat Summary ===

# of expected passes2757
# of unexpected failures26
# of expected failures  24
# of unsupported tests  25

Ada is a low priority side project for me, so if you want non-trivial changes
it may be a while before I can get to them.  There is a lot of other stuff
higher on my priority list at the moment, such as getting native gdb support
working.  If this isn't OK as is, then I'm willing to put work-in-progress
patches in a bug report or on a branch or something.

OK?

Jim

gcc/ada/
* Makefile.rtl: Add riscv*-linux* support.
* libgnarl/s-linux__riscv.ads: New.
* libgnat/system-linux-riscv.ads: New.
---
 gcc/ada/Makefile.rtl   |  28 +
 gcc/ada/libgnarl/s-linux__riscv.ads| 133 ++
 gcc/ada/libgnat/system-linux-riscv.ads | 147 +
 3 files changed, 308 insertions(+)
 create mode 100644 gcc/ada/libgnarl/s-linux__riscv.ads
 create mode 100644 gcc/ada/libgnat/system-linux-riscv.ads

diff --git a/gcc/ada/Makefile.rtl b/gcc/ada/Makefile.rtl
index f69170d9fe3..374c60b576e 100644
--- a/gcc/ada/Makefile.rtl
+++ b/gcc/ada/Makefile.rtl
@@ -2468,6 +2468,34 @@ ifeq ($(strip $(filter-out %x32 linux%,$(target_cpu) 
$(target_os))),)
   LIBRARY_VERSION := $(LIB_VERSION)
 endif
 
+# RISC-V Linux
+ifeq ($(strip $(filter-out riscv% linux%,$(target_cpu) $(target_os))),)
+  LIBGNAT_TARGET_PAIRS = \
+  a-intnam.adshttp://www.gnu.org/licenses/>.  --
+--  --
+--
+
+--  This is the RISC-V version of this package
+
+--  This package encapsulates cpu specific differences between implementations
+--  of GNU/Linux, in order to share s-osinte-linux.ads.
+
+--  PLEASE DO NOT add any with-clauses to this package or remove the pragma
+--  Preelaborate. This package is designed to be a bottom-level (leaf) package
+
+with Interfaces.C;
+
+package System.Linux is
+   pragma Preelaborate;
+
+   --
+   -- Time --
+   --
+
+   subtype int is Interfaces.C.int;
+   subtype longis Interfaces.C.long;
+   subtype suseconds_t is Interfaces.C.long;
+   subtype time_t  is Interfaces.C.long;
+   subtype clockid_t   is Interfaces.C.int;
+
+   type timespec is record
+  tv_sec  : time_t;
+  tv_nsec : long;
+   end record;
+   pragma Convention (C, timespec);
+
+   type timeval is record
+  tv_sec  : time_t;
+  tv_usec : suseconds_t;
+   end record;
+   pragma Convention (C, timeval);
+
+   ---
+   -- Errno --
+   ---
+
+   EAGAIN: constant := 11;
+   EINTR : constant := 4;
+   EINVAL: constant := 22;
+   ENOMEM: constant := 12;
+   EPERM : constant := 1;
+   ETIMEDOUT : constant := 110;
+
+   -
+   -- Signals --
+   -
+
+   SIGHUP : constant := 1; --  hangup
+   SIGINT : constant := 2; --  interrupt (rubout)
+   SIGQUIT: constant := 3; --  quit (ASCD FS)
+   SIGILL : constant := 4; --  illegal instruction (not reset)
+   SIGTRAP: constant := 5; --  trace trap (not reset)
+   SIGIOT : constant := 6; --  IOT instruction
+   SIGABRT: constant := 6; --  used by abort, replace SIGIOT in the  future
+   SIGBUS : constant := 7; --  bus error
+   SIGFPE : constant := 8; --  floating point exception
+   SIGKILL: constant := 9; --  kill (cannot be caught or ignored)
+   SIGUSR1: constant := 10; --  user defined signal 1
+   SIGSEGV: constant := 11; --  segmentation violation
+   SIGUSR2: constant := 12; --  user defined signal 2
+   SIGPIPE: constant := 13; --  write on a pipe with no one to read it
+   SIGALRM: constant := 14; --  alarm clock
+   SIGTERM: constant := 15; --  software termination signal from kill
+   SIGSTKFLT  : constant := 16; --  coprocessor stack fault (Linux)
+   SIGCLD : constant := 17; --  alias for SIGCHLD
+   SIGCHLD: constant := 17; --  child status change
+   SIGCONT: constant := 18; --  stopped process has been continued
+   SIGSTOP: constant := 19; -- 

[PATCH, Ada] Makefile patches from initial RISC-V cross/native build.

[Jim Wilson]

These are some patches I needed to complete my cross build of a native
riscv linux Ada compiler.  Some paths were different on the build machine
and host machine.  I needed to pass options into gnatmake to work around this,
and that required fixing some makefile rules to use $(GNATMAKE) instead of
calling gnatmake directly.

Tested with native riscv-linux bootstrap with Ada enabled.

OK?

Jim

gcc/ada/
* Make-generated.in (treeprs.ads): Use $(GNATMAKE) instead of gnatmake.
(einfo.h, sinfo.h, stamp-snames, stamp-nmake): Likewise.
* gcc-interface/Makefile.in (xoscons): Likewise.
---
 gcc/ada/Make-generated.in | 10 +-
 gcc/ada/gcc-interface/Makefile.in |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/gcc/ada/Make-generated.in b/gcc/ada/Make-generated.in
index 757eaa85b90..bdcb62c4e56 100644
--- a/gcc/ada/Make-generated.in
+++ b/gcc/ada/Make-generated.in
@@ -28,21 +28,21 @@ $(ADA_GEN_SUBDIR)/treeprs.ads : 
$(ADA_GEN_SUBDIR)/treeprs.adt $(ADA_GEN_SUBDIR)/
-$(MKDIR) $(ADA_GEN_SUBDIR)/bldtools/treeprs
$(RM) $(addprefix $(ADA_GEN_SUBDIR)/bldtools/treeprs/,$(notdir $^))
$(CP) $^ $(ADA_GEN_SUBDIR)/bldtools/treeprs
-   (cd $(ADA_GEN_SUBDIR)/bldtools/treeprs; gnatmake -q xtreeprs ; 
./xtreeprs treeprs.ads )
+   (cd $(ADA_GEN_SUBDIR)/bldtools/treeprs; $(GNATMAKE) -q xtreeprs ; 
./xtreeprs treeprs.ads )
$(MOVE_IF_CHANGE) $(ADA_GEN_SUBDIR)/bldtools/treeprs/treeprs.ads 
$(ADA_GEN_SUBDIR)/treeprs.ads
 
 $(ADA_GEN_SUBDIR)/einfo.h : $(ADA_GEN_SUBDIR)/einfo.ads 
$(ADA_GEN_SUBDIR)/einfo.adb $(ADA_GEN_SUBDIR)/xeinfo.adb 
$(ADA_GEN_SUBDIR)/ceinfo.adb
-$(MKDIR) $(ADA_GEN_SUBDIR)/bldtools/einfo
$(RM) $(addprefix $(ADA_GEN_SUBDIR)/bldtools/einfo/,$(notdir $^))
$(CP) $^ $(ADA_GEN_SUBDIR)/bldtools/einfo
-   (cd $(ADA_GEN_SUBDIR)/bldtools/einfo; gnatmake -q xeinfo ; ./xeinfo 
einfo.h )
+   (cd $(ADA_GEN_SUBDIR)/bldtools/einfo; $(GNATMAKE) -q xeinfo ; ./xeinfo 
einfo.h )
$(MOVE_IF_CHANGE) $(ADA_GEN_SUBDIR)/bldtools/einfo/einfo.h 
$(ADA_GEN_SUBDIR)/einfo.h
 
 $(ADA_GEN_SUBDIR)/sinfo.h : $(ADA_GEN_SUBDIR)/sinfo.ads 
$(ADA_GEN_SUBDIR)/sinfo.adb $(ADA_GEN_SUBDIR)/xsinfo.adb 
$(ADA_GEN_SUBDIR)/csinfo.adb
-$(MKDIR) $(ADA_GEN_SUBDIR)/bldtools/sinfo
$(RM) $(addprefix $(ADA_GEN_SUBDIR)/bldtools/sinfo/,$(notdir $^))
$(CP) $^ $(ADA_GEN_SUBDIR)/bldtools/sinfo
-   (cd $(ADA_GEN_SUBDIR)/bldtools/sinfo; gnatmake -q xsinfo ; ./xsinfo 
sinfo.h )
+   (cd $(ADA_GEN_SUBDIR)/bldtools/sinfo; $(GNATMAKE) -q xsinfo ; ./xsinfo 
sinfo.h )
$(MOVE_IF_CHANGE) $(ADA_GEN_SUBDIR)/bldtools/sinfo/sinfo.h 
$(ADA_GEN_SUBDIR)/sinfo.h
 
 $(ADA_GEN_SUBDIR)/snames.h $(ADA_GEN_SUBDIR)/snames.ads 
$(ADA_GEN_SUBDIR)/snames.adb : $(ADA_GEN_SUBDIR)/stamp-snames ; @true
@@ -50,7 +50,7 @@ $(ADA_GEN_SUBDIR)/stamp-snames : 
$(ADA_GEN_SUBDIR)/snames.ads-tmpl $(ADA_GEN_SUB
-$(MKDIR) $(ADA_GEN_SUBDIR)/bldtools/snamest
$(RM) $(addprefix $(ADA_GEN_SUBDIR)/bldtools/snamest/,$(notdir $^))
$(CP) $^ $(ADA_GEN_SUBDIR)/bldtools/snamest
-   (cd $(ADA_GEN_SUBDIR)/bldtools/snamest; gnatmake -q xsnamest ; 
./xsnamest )
+   (cd $(ADA_GEN_SUBDIR)/bldtools/snamest; $(GNATMAKE) -q xsnamest ; 
./xsnamest )
$(MOVE_IF_CHANGE) $(ADA_GEN_SUBDIR)/bldtools/snamest/snames.ns 
$(ADA_GEN_SUBDIR)/snames.ads
$(MOVE_IF_CHANGE) $(ADA_GEN_SUBDIR)/bldtools/snamest/snames.nb 
$(ADA_GEN_SUBDIR)/snames.adb
$(MOVE_IF_CHANGE) $(ADA_GEN_SUBDIR)/bldtools/snamest/snames.nh 
$(ADA_GEN_SUBDIR)/snames.h
@@ -61,7 +61,7 @@ $(ADA_GEN_SUBDIR)/stamp-nmake: $(ADA_GEN_SUBDIR)/sinfo.ads 
$(ADA_GEN_SUBDIR)/nma
-$(MKDIR) $(ADA_GEN_SUBDIR)/bldtools/nmake
$(RM) $(addprefix $(ADA_GEN_SUBDIR)/bldtools/nmake/,$(notdir $^))
$(CP) $^ $(ADA_GEN_SUBDIR)/bldtools/nmake
-   (cd $(ADA_GEN_SUBDIR)/bldtools/nmake; gnatmake -q xnmake ; ./xnmake -b 
nmake.adb ; ./xnmake -s nmake.ads)
+   (cd $(ADA_GEN_SUBDIR)/bldtools/nmake; $(GNATMAKE) -q xnmake ; ./xnmake 
-b nmake.adb ; ./xnmake -s nmake.ads)
$(MOVE_IF_CHANGE) $(ADA_GEN_SUBDIR)/bldtools/nmake/nmake.ads 
$(ADA_GEN_SUBDIR)/nmake.ads
$(MOVE_IF_CHANGE) $(ADA_GEN_SUBDIR)/bldtools/nmake/nmake.adb 
$(ADA_GEN_SUBDIR)/nmake.adb
touch $(ADA_GEN_SUBDIR)/stamp-nmake
diff --git a/gcc/ada/gcc-interface/Makefile.in 
b/gcc/ada/gcc-interface/Makefile.in
index 9a52e6d8edb..601f23afc1c 100644
--- a/gcc/ada/gcc-interface/Makefile.in
+++ b/gcc/ada/gcc-interface/Makefile.in
@@ -613,7 +613,7 @@ OSCONS_EXTRACT=$(OSCONS_CC) $(GNATLIBCFLAGS_FOR_C) -S 
s-oscons-tmplt.i
-$(MKDIR) ./bldtools/oscons
$(RM) $(addprefix ./bldtools/oscons/,$(notdir $^))
$(CP) $^ ./bldtools/oscons
-   (cd ./bldtools/oscons ; gnatmake -q xoscons)
+   (cd ./bldtools/oscons ; $(GNATMAKE) -q xoscons)
 
 $(RTSDIR)/s-oscons.ads: ../stamp-gnatlib1-$(RTSDIR) s-oscons-tmplt.c gsocket.h 
./bldtools/oscons/xoscons
$(RM) $(RTSD

[PATCH] RISC-V: Add support for riscv-*-*.

[Jim Wilson]

Support for riscv-* was added to config.sub upstream, so we need to handle it
in gcc configure.  Just one place needs to be fixed for now to make this work.

Tested with riscv{32,64}-{elf,linux} and riscv-elf cross builds.

Committed.

Jim

gcc/
* config.gcc (riscv*-*-*): When setting xlen, handle riscv-*.
---
 gcc/config.gcc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 63162aab676..78e84c2b864 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4091,7 +4091,7 @@ case "${target}" in
supported_defaults="abi arch tune"
 
case "${target}" in
-   riscv32*) xlen=32 ;;
+   riscv-* | riscv32*) xlen=32 ;;
riscv64*) xlen=64 ;;
*) echo "Unsupported RISC-V target ${target}" 1>&2; exit 1 ;;
esac
-- 
2.17.1

Re: [PATCH] Update config.guess and config.sub

[Jim Wilson]

On Thu, Jul 5, 2018 at 12:22 PM, Liviu Ionescu  wrote:
>> On 5 Jul 2018, at 22:17, Jim Wilson  wrote:
>> ... I can fix the gcc config.sub to make it work.
> Or you can edit `gcc/config.gcc` and trigger an error for `riscv-linux*`.

I added patches to binutils and gcc to make riscv-elf work.
riscv-rtems probably also works as a result of this.  I'm not worrying
about riscv-linux for now.  There may be more stuff that needs to be
fixed to make riscv-* work though.  I only checked binutils and gcc.

Jim

Re: [PATCH, Ada] Makefile patches from initial RISC-V cross/native build.

[Jim Wilson]

On Fri, Jul 6, 2018 at 12:18 AM, Arnaud Charlet  wrote:
>> These are some patches I needed to complete my cross build of a native
>> riscv linux Ada compiler.  Some paths were different on the build machine
>> and host machine.  I needed to pass options into gnatmake to work around 
>> this,
>> and that required fixing some makefile rules to use $(GNATMAKE) instead of
>> calling gnatmake directly.
>>
>> Tested with native riscv-linux bootstrap with Ada enabled.
>>
>> OK?
>
> OK, thanks.
>
>> Jim
>>
>>   gcc/ada/
>>   * Make-generated.in (treeprs.ads): Use $(GNATMAKE) instead of
>>   gnatmake.
>>   (einfo.h, sinfo.h, stamp-snames, stamp-nmake): Likewise.
>>   * gcc-interface/Makefile.in (xoscons): Likewise.

Committed.

Jim

Re: [PATCH, Ada] RISC-V: Initial riscv linux Ada port.

[Jim Wilson]

On Fri, Jul 6, 2018 at 12:16 AM, Arnaud Charlet  wrote:
>> Ada is a low priority side project for me, so if you want non-trivial changes
>> it may be a while before I can get to them.  There is a lot of other stuff
>> higher on my priority list at the moment, such as getting native gdb support
>> working.  If this isn't OK as is, then I'm willing to put work-in-progress
>> patches in a bug report or on a branch or something.
>>
>> OK?
>
> This is OK, thanks.
>
>> Jim
>>
>>   gcc/ada/
>>   * Makefile.rtl: Add riscv*-linux* support.
>>   * libgnarl/s-linux__riscv.ads: New.
>>   * libgnat/system-linux-riscv.ads: New.

Committed.

Jim

Re: [PATCH, Ada] Makefile patches from initial RISC-V cross/native build.

[Jim Wilson]

On Sat, Jul 7, 2018 at 12:25 AM, Andreas Schwab  wrote:
> On Jul 05 2018, Jim Wilson  wrote:
>
>> Tested with native riscv-linux bootstrap with Ada enabled.
>
> I'm getting a lot of errors from the assembler "non-constant .uleb128 is
> not supported" when trying to bootstrap the compiler with the
> cross-compiled ada compiler.

GCC configure assumes that if you have gas, then non-constant .uleb128
is OK.  However, because RISC-V deletes instructions at link time, we
cannot allow some forms of this construct.  If you have a working gas
available at gcc configure time, then it should do a gas run-time test
and discover that this gas feature does not work.  If you do not have
a working gas at gcc configure time, it will assume the feature is
available, try to use it, and then you get a build failure due to gas
errors.

The only time I've ever seen this error is if I try to use an
old-style combined-tree build approach, because in this case gcc
configures before gas is built.  If you build and install binutils,
and then build and install gcc, the build will work.  I haven't tried
to fix the old-style combined-tree approach, I didn't see an obvious
fix, and since I don't like to do builds this way anymore it wasn't
important enough to me to try to fix.

JIm

Re: [PATCH, Ada] RISC-V: Initial riscv linux Ada port.

[Jim Wilson]

On Fri, Jul 6, 2018 at 12:55 AM, Eric Botcazou  wrote:
>> Ada doesn't use trampolines if you define...
>>
>> > +   Always_Compatible_Rep : constant Boolean := False;
>>
>> ...this to False.
>
> And also define TARGET_CUSTOM_FUNCTION_DESCRIPTORS for the architecture.

I tried adding the missing definition.  I now get

=== acats Summary ===
# of expected passes2320
# of unexpected failures0

=== gnat Summary ===

# of expected passes2779
# of unexpected failures4
# of expected failures  24
# of unsupported tests  25

So yes, that solved my problem, and we have a working RISC-V Ada port
now.  Thanks for the help.

Jim

[PATCH] RISC-V: Finish Ada port.

[Jim Wilson]

Thanks to Eric Botcazou, this eliminates almost all of the remaining Ada
testsuite failures by adding a missing definition for the target specific
handling of function descriptors.

Tested with a native riscv64-linux bootstrap with Ada, and running the Ada
testsuite.  There are only 4 failures left.

Committed.

Jim

gcc/
* config/riscv/riscv.c (TARGET_CUSTOM_FUNCTION_DESCRIPTORS): New.
---
 gcc/config/riscv/riscv.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index d87836f53f8..218f4de7d41 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -4786,6 +4786,10 @@ riscv_constant_alignment (const_tree exp, HOST_WIDE_INT 
align)
 #undef TARGET_WARN_FUNC_RETURN
 #define TARGET_WARN_FUNC_RETURN riscv_warn_func_return
 
+/* The low bit is ignored by jump instructions so is safe to use.  */
+#undef TARGET_CUSTOM_FUNCTION_DESCRIPTORS
+#define TARGET_CUSTOM_FUNCTION_DESCRIPTORS 1
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-riscv.h"
-- 
2.17.1

Re: [PATCH, Ada] RISC-V: Initial riscv linux Ada port.

[Jim Wilson]

On Sat, Jul 7, 2018 at 9:41 AM, Eric Botcazou  wrote:
> You're welcome.  Are the 4 remaining failures related to stack checking?

FAIL: gnat.dg/debug11.adb scan-assembler-times 0x5a.*DW_AT_discr_list 1
FAIL: gnat.dg/debug11.adb scan-assembler-times 0x80.*DW_AT_discr_list 1
FAIL: gnat.dg/trampoline4.adb scan-assembler GNU-stack.*x
FAIL: gnat.dg/warn5.adb (test for excess errors)

I haven't tried looking at the failures yet, and might not spend much
more time on this.  Two of them are debug related, and debug support
is a work in progress.  I need to finish the native riscv64-linux
support before we can do anything useful there, and I'd like to get
back to working on that as soon as possible.  The GNU-stack error
looks a little worrisome.  I'd expect a linux port to get GNU-stack
stuff right without much trouble.  The last one is
warn5.adb:29:30: warning: source alignment (4) < alignment of "Element_Type" (8)
Maybe something I copied from the mips linux port is wrong for riscv64 linux.

Jim

Re: [PATCH, Ada] Makefile patches from initial RISC-V cross/native build.

[Jim Wilson]

On Sat, Jul 7, 2018 at 10:06 AM, Andreas Schwab  wrote:
> On Jul 07 2018, Jim Wilson  wrote:
>
>> If you build and install binutils, and then build and install gcc, the
>> build will work.
>
> Not if the compiler was built in a canadian cross.  That's the only way
> to bootstrap an Ada compiler.

It worked for me.  I have RISC-V Fedora Linux Ada compiler binaries
that I built cross from an x86_64 Ubuntu 18.04 system.

You should only see the gas error if your gas sources are in the same
source tree as your gcc sources.  if you have separate binutils and
gcc source trees, the build should work.  In the gcc/configure.ac
file, see the in_tree_gas and in_tree_gas_elf variables, which are set
if gas/ELF gas are in the same source tree.  And then look at
configure, and note that configure makes assumptions about gas
features when these variables are set, instead of doing run-time
assembler checks.

I've done number of canadian cross compiler builds, and I've never run
into this gas problem in any of them, but I always use separate
binutils and gcc source trees for my builds.

Jim

Re: [PATCH, Ada] Makefile patches from initial RISC-V cross/native build.

[Jim Wilson]

On Sat, Jul 7, 2018 at 11:30 AM, Andreas Schwab  wrote:
> On Jul 07 2018, Jim Wilson  wrote:
>> if you have separate binutils and gcc source trees, the build should
>> work.
>
> It's not the canadian cross build that fails, but the subsequent native
> build using the (misconfigured) canadian cross build.

This works for me.  I have a sysroot created from a live system.  I
configure/build/install a cross binutils, then cross gcc.  I put the
cross install tree on my path, then I configure/build/install the
canadian cross binutils and then gcc.  Then I copy the canadian cross
install tree to the RISC-V system and use it to bootstrap a native gcc
with Ada.

This is also presumably what the debian, fedora, and gentoo folks did
to get their first native compiler.  They didn't report any problems.

Is this the first time you are trying to build a native RISC-V
compiler for OpenSuse?  If you already built a C/C++ compiler, then I
would expect the Ada compiler build to work the same way.  But if this
is your first native RISC-V compiler build attempt then maybe there is
something different about how you are trying to do the build.  I could
share my build scripts with you but they are pretty simple.  It is
also possible to build a canadian cross compiler from github
riscv/riscv-gnu-toolchain using the linux-native makefile rule, but
since it has its own copy of glibc you might run into glibc versioning
problems.

One thing I could mention is that I started with a gcc-7.3 release,
because Ubuntu 18.04 has gcc-7.3 as the native compiler, and I wasn't
sure if gcc-7 Ada could build gcc-9 Ada.  Sometimes old Ada versions
can't compile new Ada versions.  So I actually did the canadian cross
build with binutils-2.30 and gcc-7.3.1, and then moved up from gcc-7
to gcc-9 on the RISC-V side.  I maybe haven't tried a canadian cross
build with a newer gcc version, so maybe something broke after gcc-7.
That seems unlikely though.

If you can give me details about exactly how you are trying to
canadian cross build the Ada compiler, I can try to reproduce your
problem here.

Jim

Re: [PATCH, Ada] Makefile patches from initial RISC-V cross/native build.

[Jim Wilson]

On Sat, Jul 7, 2018 at 2:43 PM, Andreas Schwab  wrote:
> On Jul 07 2018, Jim Wilson  wrote:
>
>> This is also presumably what the debian, fedora, and gentoo folks did
>> to get their first native compiler.  They didn't report any problems.
>
> Of course, they didn't build an ada compiler.

But building an Ada compiler works exactly the same as building C and
C++ compilers.  There should really be no difference.

>> Is this the first time you are trying to build a native RISC-V
>> compiler for OpenSuse?
>
> I have bootstrapped several architectures for SUSE and openSUSE, thank
> you.

I am aware that you have much experience building stuff.  But building
RISC-V is a little different than the others, because of link-time
relaxations deleting code, which means some things that work for other
targets don't work for RISC-V.  It is possible that the scheme you are
using to build canadian cross compilers will work for other targets
but not RISC-V because of this problem.  That is why I asked if this
is your first attempt to canadian cross build a RISC-V native
compiler.  And because of the above, that building Ada should be no
different from building C and C++.

Anyways, all I can repeat is what I've said before.  It works for me.
I can give you my trivial build scripts if you want.  Otherwise, you
have to give me enough info that I can reproduce the problem that you
ran into.

Jim

Re: [PATCH] RISC-V: Report error if function declare with different

[Jim Wilson]

On Thu, Jul 12, 2018 at 7:53 AM, Kito Cheng  wrote:
> ping.
> On Fri, Jul 6, 2018 at 4:38 PM Kito Cheng  wrote:
>>
>> Hi all:
>>
>> This patch implemented TARGET_MERGE_DECL_ATTRIBUTES hook to check the
>> interrupter is all compatible, tested with rv32ima and rv64ima elf
>> toolchain.
>>
>> gcc/ChangeLog
>> 2018-07-06  Kito Cheng  
>>
>> * config/riscv/riscv.c (enum riscv_privilege_levels): Add 
>> UNKNOWN_MODE.
>> (riscv_expand_epilogue): Add more assertion to check interrupt mode.
>> (riscv_set_current_function): Extract getting interrupt type to new
>> function.
>> (riscv_get_interrupt_type): New function.
>> (riscv_merge_decl_attributes): New function, checking interrupt type 
>> is
>> same.
>> (TARGET_MERGE_DECL_ATTRIBUTES): Define.
>>
>> gcc/testsuite/ChangeLog
>> 2018-07-06  Kito Cheng  
>>
>> * gcc.target/riscv/interrupt-conflict-mode.c: New.

Ok.

Committed.

Jim

[PATCH] RISC-V: Silence expected Ada testsuite warning.

[Jim Wilson]

This eliminates one Ada testsuite failure, by adding riscv*-*-* for the list
of targets that expect this warning.  This was pre-approved by Eric Botcazou.

Tested with a riscv64-linux native Ada testsuite run.  There was one less
failure with this patch.

Committed.

Jim

gcc/testsuite/
* gnat.dg/warn5.adb: Expect warning for riscv*-*-*.
---
 gcc/testsuite/gnat.dg/warn5.adb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gnat.dg/warn5.adb b/gcc/testsuite/gnat.dg/warn5.adb
index 77e4a66f733..61f76972480 100644
--- a/gcc/testsuite/gnat.dg/warn5.adb
+++ b/gcc/testsuite/gnat.dg/warn5.adb
@@ -26,7 +26,7 @@ procedure Warn5 is
 
   function Pointer (Pos : Natural; List : List_Type) return Pointer_Type is
   begin
-return To_Ptr(List.A(Pos)'Address); -- { dg-warning "source alignment" "" 
{ target alpha*-*-* arm*-*-* hppa*-*-* ia64-*-* mips*-*-* sparc*-*-* } }
+return To_Ptr(List.A(Pos)'Address); -- { dg-warning "source alignment" "" 
{ target alpha*-*-* arm*-*-* hppa*-*-* ia64-*-* mips*-*-* riscv*-*-* sparc*-*-* 
} }
   end;
 
 begin
-- 
2.17.1

Re: [PATCH, Ada] RISC-V: Initial riscv linux Ada port.

[Jim Wilson]

On Thu, Jul 12, 2018 at 8:56 AM, Pierre-Marie de Rodat
 wrote:
> I don’t have much more to say than debug11.adb’s comment ;-)
>
>> This testcase checks that in the DWARF description of the variant type
>> below, the C discriminant is properly described as unsigned, hence the
>> 0x5a
>> ('Z') and 0x80 (128) values in the DW_AT_discr_list attribute. If it was
>> described as signed, we would have instead 90 and -128.
>
> I don’t have an Ada RISC-V compiler (nor binutils) to check right now: would
> it be possible to send the corresponding debug11.s and debug11.o? Hopefully
> we just have to enhance the regexps.

I poked at this a little and noticed a difference between the x86_64
support and the RISC-V support.  The RISC-V C language port has char
as unsigned by default.  The x86_64 port has char signed by default.
If I add a -fsigned-char option, then the testcase works as expected
for RISC-V.  Curiously, the Ada compiler accepts -fsigned-char but not
-funsigned-char.  I tried hacking in a -funsigned-char flag, but when
I use it with the x86_64 port the result is still correct.  Maybe my
quick hack wasn't quite right.  Anyways, the default signedness of
char has something to do with the problem.

Jim

[PATCH] RISC-V: Fix nested function trampolines.

[Jim Wilson]

This fixes an oversight.  I suspect that there was a RISC-V QEMU bug allowing
stacks to be executable by default, so we didn't see the problem until we
started running the gcc testsuite on hardware.  This was also confused by the
late ABI change to the cache flush support when upstreaming the glibc support.
And the linux kernel bug we just found a couple of days ago.  Anyways,
trampolines work now, if you have all of the right patches.

This was tested with a native riscv64-linux bootstrap and make check.  It
fixes 69 gcc failures, 49 gfortran failures, and 1 objc failure.  It also fixes
one gnat failure, which is how I happened to notice that gcc was broken too.

Kito/Ruslan - You may need a similar change to the config/riscv-freebsd.h file.
I don't have a setup where I can test that.

Committed.

Jim

gcc/
* config/riscv/linux.h (TARGET_ASM_FILE_END): New.
---
 gcc/config/riscv/linux.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gcc/config/riscv/linux.h b/gcc/config/riscv/linux.h
index 85561846dad..e208c95fe13 100644
--- a/gcc/config/riscv/linux.h
+++ b/gcc/config/riscv/linux.h
@@ -66,3 +66,5 @@ along with GCC; see the file COPYING3.  If not see
   %{rdynamic:-export-dynamic} \
   -dynamic-linker " GNU_USER_DYNAMIC_LINKER "} \
 %{static:-static}}"
+
+#define TARGET_ASM_FILE_END file_end_indicate_exec_stack
-- 
2.17.1

Re: [PATCH] Add baseline symbols for riscv64-linux-gnu

[Jim Wilson]

On Mon, Jul 16, 2018 at 3:24 AM, Andreas Schwab  wrote:
> * config/abi/post/riscv64-linux-gnu/baseline_symbols.txt: New file.
>

I'm not familiar with the details of these baseline symbol files.

When I try running "make new-abi-baseline" on my Fedora riscv64-linux
system using top of tree, I get 35 extra lines in the
baseline_symbols.txt.new file.  It looks like 33 of them are because I
have
OBJECT:0:GLIBCXX_3.4.26
and yours only goes up to 3.4.25.  The other two are TLS entries.
> TLS:8:_ZSt11__once_call@@GLIBCXX_3.4.11
> TLS:8:_ZSt15__once_callable@@GLIBCXX_3.4.11
Not sure why there are no TLS entries in your file, and two in mine.
The testsuite does pass with a message about 33 added symbols and 2
undesignated symbols which are the TLS symbols.

Anyways, it passed, so your patch is OK.

Jim

Re: [PATCH] RISC-V: Fix -march option parsing when `p` extension exists.

[Jim Wilson]

On Thu, Jan 21, 2021 at 10:49 PM Kito Cheng  wrote:

> I think this patch is small enough to accept without FSF copyright
> assignment, and he is also on the way of the process, what do you
> think?
>

I see the copyright assignments on file at the FSF when I checked today.

Jim

Re: [RFC] test builtin ratio for loop distribution

[Jim Wilson]

On Wed, Jan 27, 2021 at 4:40 AM Alexandre Oliva  wrote:

> This patch attempts to fix a libgcc codegen regression introduced in
> gcc-10, as -ftree-loop-distribute-patterns was enabled at -O2.
>
> RISC-V doesn't have any setmemM pattern, so the loops above end up
> "optimized" into memset calls, incurring not only the overhead of an
> explicit call, but also discarding the information the compiler has
> about the alignment of the destination, and that the length is a
> multiple of the word alignment.
>
> FYI we have a bug report for this for a coremark regression which sounds
like the same problem.
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94092

Jim

Re: [PATCH] PR target/98878 - Incorrect multilib list for riscv*-rtems

[Jim Wilson]

On Thu, Feb 4, 2021 at 2:02 AM Kito Cheng  wrote:

> * gcc.c (print_multilib_info): Check all required argument is
> provided
> by default arg.
>

This looks OK to me, but...

>
> -  /* If this directory requires any default arguments, we can skip
> +  /* If this directory requires any default arguments, and any default
> +arguments not appear in the ! argument list, then we can skip
>  it.  We will already have printed a directory identical to
>  this one which does not require that default argument.  */
>

I would suggest "any default arguments not appear in the ! argument list"
to be instead "no default arguments appear in the ! argument list".  But
there are actually 3 conditions being tested, and the comment only mentions
2.  The other condition is that a required argument must appear in the
default argument list.  That can be combined with the first condition by
saying all required arguments must be default arguments.  So maybe
something like

/* If all required arguments are default arguments, and no default
   arguments appear in the ! argument list, then we can skip it.

Jim

[PATCH] RISC-V: Shorten memrefs improvement, partial fix 97417.

[Jim Wilson]

We already have a check for riscv_shorten_memrefs in riscv_address_cost.
This adds the same check to riscv_rtx_costs.  Making this work also
requires a change to riscv_compressed_lw_address_p to work before reload
by checking the offset and assuming any pseudo reg is OK.  Testing shows
that this consistently gives small code size reductions.

Tested with riscv32-elf rv32imac/ilp32 and riscv64-linux rv64gc/lp64d
builds and checks and there were no regressions.

Committed.

gcc/
PR target/97417
* config/riscv/riscv.c (riscv_compressed_lw_address_p): Drop early
exit when !reload_completed.  Only perform check for compressed reg
if reload_completed.
(riscv_rtx_costs): In MEM case, when optimizing for size and
shorten memrefs, if not compressible, then increase cost.
---
 gcc/config/riscv/riscv.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index ff41795a031..7d274596ba3 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -891,17 +891,13 @@ riscv_compressed_lw_address_p (rtx x)
   bool result = riscv_classify_address (&addr, x, GET_MODE (x),
reload_completed);
 
-  /* Before reload, assuming all load/stores of valid addresses get compressed
- gives better code size than checking if the address is reg + small_offset
- early on.  */
-  if (result && !reload_completed)
-return true;
-
   /* Return false if address is not compressed_reg + small_offset.  */
   if (!result
   || addr.type != ADDRESS_REG
-  || (!riscv_compressed_reg_p (REGNO (addr.reg))
-   && addr.reg != stack_pointer_rtx)
+  /* Before reload, assume all registers are OK.  */
+  || (reload_completed
+ && !riscv_compressed_reg_p (REGNO (addr.reg))
+ && addr.reg != stack_pointer_rtx)
   || !riscv_compressed_lw_offset_p (addr.offset))
 return false;
 
@@ -1708,6 +1704,13 @@ riscv_rtx_costs (rtx x, machine_mode mode, int 
outer_code, int opno ATTRIBUTE_UN
 instructions it needs.  */
   if ((cost = riscv_address_insns (XEXP (x, 0), mode, true)) > 0)
{
+ /* When optimizing for size, make uncompressible 32-bit addresses
+more expensive so that compressible 32-bit addresses are
+preferred.  */
+ if (TARGET_RVC && !speed && riscv_mshorten_memrefs && mode == SImode
+ && !riscv_compressed_lw_address_p (XEXP (x, 0)))
+   cost++;
+
  *total = COSTS_N_INSNS (cost + tune_param->memory_cost);
  return true;
}
-- 
2.17.1

[PATCH] RISC-V: Avoid zero/sign extend for volatile loads. Fix for 97417.

[Jim Wilson]

From: Levy Hsu 

This expands sub-word loads as a zero/sign extended load, followed by
a subreg.  This helps eliminate unnecessary zero/sign extend insns after
the load, particularly for volatiles, but also in some other cases.
Testing shows that it gives consistent code size decreases.

Tested with riscv32-elf rv32imac/ilp32 and riscv64-linux rv64gc/lp064d
builds and checks.  Some -gsplit-stack tests fail with the patch, but
this turns out to be an existing bug with the split-stack support that
I hadn't noticed before.  It isn't a bug in this patch.  Ignoring that
there are no regressions.

Committed.

gcc/
PR target/97417
* config/riscv/riscv-shorten-memrefs.c (pass_shorten_memrefs): Add
extend parameter to get_si_mem_base_reg declaration.
(get_si_mem_base_reg): Add extend parameter.  Set it.
(analyze): Pass extend arg to get_si_mem_base_reg.
(transform): Likewise.  Use it when rewriting mems.
* config/riscv/riscv.c (riscv_legitimize_move): Check for subword
loads and emit sign/zero extending load followed by subreg move.
---
 gcc/config/riscv/riscv-shorten-memrefs.c | 34 +++-
 gcc/config/riscv/riscv.c | 22 +++
 2 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/gcc/config/riscv/riscv-shorten-memrefs.c 
b/gcc/config/riscv/riscv-shorten-memrefs.c
index b1b57f1b5e0..3f34065c1ce 100644
--- a/gcc/config/riscv/riscv-shorten-memrefs.c
+++ b/gcc/config/riscv/riscv-shorten-memrefs.c
@@ -75,12 +75,19 @@ private:
 
   regno_map * analyze (basic_block bb);
   void transform (regno_map *m, basic_block bb);
-  bool get_si_mem_base_reg (rtx mem, rtx *addr);
+  bool get_si_mem_base_reg (rtx mem, rtx *addr, bool *extend);
 }; // class pass_shorten_memrefs
 
 bool
-pass_shorten_memrefs::get_si_mem_base_reg (rtx mem, rtx *addr)
+pass_shorten_memrefs::get_si_mem_base_reg (rtx mem, rtx *addr, bool *extend)
 {
+  /* Whether it's sign/zero extended.  */
+  if (GET_CODE (mem) == ZERO_EXTEND || GET_CODE (mem) == SIGN_EXTEND)
+{
+  *extend = true;
+  mem = XEXP (mem, 0);
+}
+
   if (!MEM_P (mem) || GET_MODE (mem) != SImode)
 return false;
   *addr = XEXP (mem, 0);
@@ -110,7 +117,8 @@ pass_shorten_memrefs::analyze (basic_block bb)
{
  rtx mem = XEXP (pat, i);
  rtx addr;
- if (get_si_mem_base_reg (mem, &addr))
+ bool extend = false;
+ if (get_si_mem_base_reg (mem, &addr, &extend))
{
  HOST_WIDE_INT regno = REGNO (XEXP (addr, 0));
  /* Do not count store zero as these cannot be compressed.  */
@@ -150,7 +158,8 @@ pass_shorten_memrefs::transform (regno_map *m, basic_block 
bb)
{
  rtx mem = XEXP (pat, i);
  rtx addr;
- if (get_si_mem_base_reg (mem, &addr))
+ bool extend = false;
+ if (get_si_mem_base_reg (mem, &addr, &extend))
{
  HOST_WIDE_INT regno = REGNO (XEXP (addr, 0));
  /* Do not transform store zero as these cannot be compressed.  */
@@ -161,9 +170,20 @@ pass_shorten_memrefs::transform (regno_map *m, basic_block 
bb)
}
  if (m->get_or_insert (regno) > 3)
{
- addr
-   = targetm.legitimize_address (addr, addr, GET_MODE (mem));
- XEXP (pat, i) = replace_equiv_address (mem, addr);
+ if (extend)
+   {
+ addr
+   = targetm.legitimize_address (addr, addr,
+ GET_MODE (XEXP (mem, 0)));
+ XEXP (XEXP (pat, i), 0)
+   = replace_equiv_address (XEXP (mem, 0), addr);
+   }
+ else
+   {
+ addr = targetm.legitimize_address (addr, addr,
+GET_MODE (mem));
+ XEXP (pat, i) = replace_equiv_address (mem, addr);
+   }
  df_insn_rescan (insn);
}
}
diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index 7d274596ba3..fffd0814eee 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -1524,6 +1524,28 @@ riscv_legitimize_const_move (machine_mode mode, rtx 
dest, rtx src)
 bool
 riscv_legitimize_move (machine_mode mode, rtx dest, rtx src)
 {
+  /* Expand 
+   (set (reg:QI target) (mem:QI (address))) 
+ to
+   (set (reg:DI temp) (zero_extend:DI (mem:QI (address
+   (set (reg:QI target) (subreg:QI (reg:DI temp) 0))
+ with auto-sign/zero extend.  */
+  if (GET_MODE_CLASS (mode) == MODE_INT
+  && GET_MODE_SIZE (mode) < UNITS_PER_WORD
+  && can_create_pseudo_p ()
+  && MEM_P (src))
+{
+  rtx temp_reg;
+  int zero_extend_p;
+
+  temp_reg = gen_reg_rtx (word_mode);
+  zero_extend_p = (LOAD_EXTEND_OP (mode) 

Re: [PATCH] RISC-V: Add implementation for builtin overflow

[Jim Wilson]

On Thu, Jan 21, 2021 at 2:25 AM Levy  wrote:

> Added implementation for builtin overflow detection, new patterns are
> listed below.
>

For rv32 SImode and rv64 DImode, the unsigned add/sub and signed/unsigned
mul patterns seem to give the same result as the default code generation.
That has me wondering if we really need the patterns.

For rv64 SImode, the signed add/sub patterns are generating worse code. Signed
add overflow without the pattern is
add a1,a0,a1
sext.w  a0,a1
bne a1,a0,.L23
and with the pattern is
mv  a5,a0
addwa0,a0,a1
slt a5,a0,a5
sltia1,a1,0
bne a1,a5,.L27
ignoring the register allocation issue we have one more instruction using the
pattern which is undesirable.  This needs a fix.  We could just use X instead
of GPR.  We could then optionally add rv64 SImode patterns.  If we don't
add them it looks like a bug, so I think we should add another pattern for
this.

For rv64 SImode, the unsigned add/sub patterns are generating better code.  We
have unnecessary zero extends without the patterns.  This suggests that the
unsigned add/sub patterns really are useful.  Only 1 of the 3 expansions is
useful, but if we only implement one of the 3 expansions that looks like a
bug so I think this is OK as is.

The signed/unsigned mul patterns only support rv32 SImode and rv64 DImode.
The rv64 SImode support is missing.  But even though there is no pattern
for it we can still get worse code for it.  An unpatched tree for signed
mul gives
mul a1,a0,a1
sext.w  a0,a1
bne a1,a0,.L37
and a patched tree gives
mul a0,a0,a1
sraiw   a4,a0,31
sraia5,a0,32
bne a4,a5,.L43
On the other hand, an unpatched tree for unsigned mul gives
sllia0,a0,32
sllia1,a1,32
srlia0,a0,32
srlia1,a1,32
mul a5,a0,a1
sllia4,a5,32
srlia4,a4,32
bne a5,a4,.L61
and a patched tree gives
sllia0,a0,32
sllia1,a1,32
srlia0,a0,32
srlia1,a1,32
mul a0,a0,a1
sraia5,a0,32
bne a5,zero,.L69
which is one instruction shorter.  In both cases, the difference is due to
the hook to set min precision to 32.  Presumably we need this hook for
add/sub, so we need to add the missing rv64 SImode signed mul pattern.
Since we have to have one of them, we may as well have all of them for
completeness.

There are a few minor style issues.

In the define_expands, the patterns aren't formatted properly.  An open
parenthesis should be aligned to an open paren at the same depth on the
line above.  If you are an emacs user, you can use M-x emacs-lisp-mode and
hit tab to align them.

In a define_expand the constraints aren't useful and shouldn't be
included.  In a define_expand that always emits its own RTL the pattern
isn't useful either.  Only the operand modes, numbers, and predicates are
useful.  This is why some define_expands only list the operands and don't
include the pattern.  But including the pattern is OK, it just isn't
required.  So the only real fix we need here is to drop the constraints.

I attached the testcase I used for testing purposes.  Every function should
be same size or smaller with the patch.

Jim
#include 

int
sub_add_p (int i, int j)
{
  int k;
  return __builtin_add_overflow_p (i, j, k);
}

int
sub_sub_p (int i, int j)
{
  int k;
  return __builtin_sub_overflow_p (i, j, k);
}

int
sub_mul_p (int i, int j)
{
  int k;
  return __builtin_mul_overflow_p (i, j, k);
}

long
sub_add_p_long (long i, long j)
{
  long k;
  return __builtin_add_overflow_p (i, j, k);
}

long
sub_sub_p_long (long i, long j)
{
  long k;
  return __builtin_sub_overflow_p (i, j, k);
}

long
sub_mul_p_long (long i, long j)
{
  long k;
  return __builtin_mul_overflow_p (i, j, k);
}

int
sub_add (int i, int j)
{
  int k;
  if (__builtin_sadd_overflow (i, j, &k))
abort ();
  return k;
}

int
sub_sub (int i, int j)
{
  int k;
  if (__builtin_ssub_overflow (i, j, &k))
abort ();
  return k;
}

int
sub_mul (int i, int j)
{
  int k;
  if (__builtin_smul_overflow (i, j, &k))
abort ();
  return k;
}

int
sub_uadd (int i, int j)
{
  int k;
  if (__builtin_uadd_overflow (i, j, &k))
abort ();
  return k;
}

int
sub_usub (int i, int j)
{
  int k;
  if (__builtin_usub_overflow (i, j, &k))
abort ();
  return k;
}

int
sub_umul (int i, int j)
{
  int k;
  if (__builtin_umul_overflow (i, j, &k))
abort ();
  return k;
}

long
sub_add_long (long i, long j)
{
  long k;
  if (__builtin_saddl_overflow (i, j, &k))
abort ();
  return k;
}

long
sub_sub_long (long i, long j)
{
  long k;
  if (__builtin_ssubl_overflow (i, j, &k))
abort ();
  return k;
}

long
sub_mul_long (long i, long j)
{
  long k;
  if (__builtin_smull_overflow (i, j, &k))
abort ();
  return k;
}

long
sub_uadd_long (long i, long j)
{
  long k;
  if (__built

Re: [PATCH] RISC-V: Zihintpause: add __builtin_riscv_pause

[Jim Wilson]

On Thu, Jan 7, 2021 at 12:50 AM Kito Cheng  wrote:

> My point is tracking info and consistent behavior/scheme with other
> extensions, so personally I strongly prefer it should be guarded with
> -march.
>

It is a hint.  We should allow it even if the architecture extension is not
enabled.

For comparison, I suggest you look at the aarch64 port.  They have 3 kinds
of hints: branch protection (bti), pointer authentication, and speculation
control.  They deliberately allow you to emit the instructions even if the
hardware doesn't support the feature because they are hints, and execute as
nops if the hardware support is missing.  The rationale is that the code
will work with or without the hardware support, but will work better with
the hardware support, so it is best to always allow it.  We should do the
same with RISC-V hints.

I agree that we need to include LLVM folks in the discussion to make sure
that GCC and LLVM handle this the same way.

Jim

Re: add rv64im{,c,fc} multilibs

[Jim Wilson]

On Tue, Feb 23, 2021 at 2:17 AM Alexandre Oliva  wrote:

> I take your response as confirming my expectation that the defaults are
> to remain unchanged for now, and I will thus proceed under this
> assumption.
>

If we add default multilibs for you, then to be fair, we need to add
default multilibs for other people that ask, and before long we are trying
to build hundreds or maybe even thousands of multilibs by default which is
unworkable.  There are simply too many different extensions, and too many
different valid combinations of them.  A problem that is quickly getting
worse, as there are a slew of extensions that are planned for final
approval this year.  The current set was defined before I started doing
RISC-V work over 3 years ago, and I've been saying no to everyone that
wants to change the default set.  The current set is tractable for newbies
to try to build and use.  People that want a different set can define their
own, and we have made it easy for people to define their own sets of
multilibs as Kito pointed out.

I do think that when the architecture profiles are adopted and implemented
it would make sense to add them to the default set, and maybe eventually
replace the default set.

Jim

Re: [PATCH] RISC-V: Define __riscv_cmodel_medany for PIC mode.

[Jim Wilson]

On Thu, Sep 24, 2020 at 10:46 PM Kito Cheng  wrote:
>
>  - According the conclusion in RISC-V C API document, we decide to deprecat
>the __riscv_cmodel_pic marco
>
>  - __riscv_cmodel_pic is deprecated and will removed in next GCC
>release.

Looks good to me.  By the way, you can self approve patches like this.

Optionally, you might add a comment to the code to point out that
__riscv_cmodel_pic is deprecated.  That makes it a little easier to
understand the code.

Jim

Re: [PATCH] RISC-V/libgcc: Use `-fasynchronous-unwind-tables' for LIB2_DIVMOD_FUNCS

[Jim Wilson]

On Sun, Aug 30, 2020 at 11:39 PM Kito Cheng  wrote
> Hi Maciej:
> LGTM, thanks for your patch!

I don't see this patch in the FSF GCC tree.  Maciej are you going to
commit it?  Or do you want us to commit it for you?

Jim

[PATCH] Fix GCC 10+ build failure with zstd version 1.2.0 or older.

[Jim Wilson]

Extends the configure check for zstd.h to also verify the zstd version,
since gcc requires features that only exist in 1.3.0 and newer.  Without
this patch we get a build error for lto-compress.c when using an old zstd
version.

Tested with builds using zstd 0.5.1, 1.2.0, 1.3.0, and 1.3.3, and checking
to see whether zstd was enabled for the build or not.

OK?

Jim

gcc/
PR 97183
* configure.ac (gcc_cv_header_zstd_h): Check ZSTD_VERISON_NUMBER.
* configure: Regenerated.
---
 gcc/configure| 11 ---
 gcc/configure.ac |  7 ++-
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/gcc/configure b/gcc/configure
index 33a3e34029f..b05a3714fb2 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -10022,9 +10022,14 @@ $as_echo_n "checking for zstd.h... " >&6; }
 if ${gcc_cv_header_zstd_h+:} false; then :
   $as_echo_n "(cached) " >&6
 else
-  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+  # We require version 1.3.0 or later.  This is the first version that has
+# ZSTD_getFrameContentSize.
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 #include 
+#if ZSTD_VERSION_NUMBER < 10300
+#error "need zstd 1.3.0 or better"
+#endif
 int
 main ()
 {
@@ -19013,7 +19018,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 19016 "configure"
+#line 19021 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -19119,7 +19124,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 19122 "configure"
+#line 19127 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 975f6d97c4b..f5612161dcd 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -1376,8 +1376,13 @@ LDFLAGS="$LDFLAGS $ZSTD_LDFLAGS"
 
 AC_MSG_CHECKING(for zstd.h)
 AC_CACHE_VAL(gcc_cv_header_zstd_h,
+# We require version 1.3.0 or later.  This is the first version that has
+# ZSTD_getFrameContentSize.
 [AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
-[[#include ]])],
+[[#include 
+#if ZSTD_VERSION_NUMBER < 10300
+#error "need zstd 1.3.0 or better"
+#endif]])],
   [gcc_cv_header_zstd_h=yes],
   [gcc_cv_header_zstd_h=no])])
 AC_MSG_RESULT($gcc_cv_header_zstd_h)
-- 
2.17.1

Re: [PATCH] Fix GCC 10+ build failure with zstd version 1.2.0 or older.

[Jim Wilson]

On Tue, Sep 29, 2020 at 1:20 AM Richard Biener
 wrote:
>
> On Tue, Sep 29, 2020 at 2:46 AM Jim Wilson  wrote:
> >
> > Extends the configure check for zstd.h to also verify the zstd version,
> > since gcc requires features that only exist in 1.3.0 and newer.  Without
> > this patch we get a build error for lto-compress.c when using an old zstd
> > version.
> >
> > Tested with builds using zstd 0.5.1, 1.2.0, 1.3.0, and 1.3.3, and checking
> > to see whether zstd was enabled for the build or not.
> >
> > OK?
>
> OK.

Thanks.  Committed to the mainline.  Presumably we want a back port to
the gcc-10 branch as the same problem is present there.  I will send a
patch for that.

Jim

[PATCH][GCC 10] Fix build failure with zstd versio9n 1.2.0 or older.

[Jim Wilson]

This is the gcc-10 branch version of the patch on mainline.

Extends the configure check for zstd.h to also verify the zstd version,
since gcc requires features that only exist in 1.3.0 and newer.  Without
this patch we get a build error for lto-compress.c when using an old zstd
version.

OK?

Jim

Backported from master:
2020-09-29  Jim Wilson  

gcc/
PR bootstrap/97183
* configure.ac (gcc_cv_header_zstd_h): Check ZSTD_VERISON_NUMBER.
* configure: Regenerated.
---
 gcc/configure| 11 ---
 gcc/configure.ac |  7 ++-
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/gcc/configure b/gcc/configure
index eb6061c1631..b4088d8fd1e 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -10024,9 +10024,14 @@ $as_echo_n "checking for zstd.h... " >&6; }
 if ${gcc_cv_header_zstd_h+:} false; then :
   $as_echo_n "(cached) " >&6
 else
-  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+  # We require version 1.3.0 or later.  This is the first version that has
+# ZSTD_getFrameContentSize.
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 #include 
+#if ZSTD_VERSION_NUMBER < 10300
+#error "need zstd 1.3.0 or better"
+#endif
 int
 main ()
 {
@@ -19015,7 +19020,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 19018 "configure"
+#line 19023 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -19121,7 +19126,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 19124 "configure"
+#line 19129 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 715fcba0482..070b9c6c497 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -1382,8 +1382,13 @@ LDFLAGS="$LDFLAGS $ZSTD_LDFLAGS"
 
 AC_MSG_CHECKING(for zstd.h)
 AC_CACHE_VAL(gcc_cv_header_zstd_h,
+# We require version 1.3.0 or later.  This is the first version that has
+# ZSTD_getFrameContentSize.
 [AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
-[[#include ]])],
+[[#include 
+#if ZSTD_VERSION_NUMBER < 10300
+#error "need zstd 1.3.0 or better"
+#endif]])],
   [gcc_cv_header_zstd_h=yes],
   [gcc_cv_header_zstd_h=no])])
 AC_MSG_RESULT($gcc_cv_header_zstd_h)
-- 
2.17.1

Re: [RISC-V] Add support for AddressSanitizer on RISC-V GCC

[Jim Wilson]

On Tue, Aug 25, 2020 at 12:39 PM Jim Wilson  wrote:
> On Wed, Aug 19, 2020 at 1:02 AM Joshua via Gcc-patches
>  wrote:
> > * config/riscv/riscv.c (asan_shadow_offset): Implement the offset 
> > of asan shadow memory for risc-v.
> > (asan_shadow_offset): new macro definition.
>
> When I try the patch, I get asan errors complaining about memory mappings.

I tried looking at this again today.  I spent a few hours debugging
sanitizer code to see what is going on, and managed to convince myself
that the patch can't possibly work for riscv64-linux.  There are at
least two changes missing.  It also can't possibly work for
riscv32-linux.  There is at least one change missing.
libsanitizer/configure.tgt only supports riscv64-linux, so nothing
gets built for riscv32-linux.  I have no idea how you got this
working.  Maybe you forgot to send the entire patch?  If there is a
way to get it working, it would be good if you could describe in
detail how you got it working.

For riscv64-linux with these additional patches

hifiveu017:1192$ more tmp.file
diff --git a/libsanitizer/asan/asan_mapping.h b/libsanitizer/asan/asan_mapping.h
index 09be904270c..906fb1eebc8 100644
--- a/libsanitizer/asan/asan_mapping.h
+++ b/libsanitizer/asan/asan_mapping.h
@@ -164,6 +164,7 @@ static const u64 kAArch64_ShadowOffset64 = 1ULL << 36;
 static const u64 kMIPS32_ShadowOffset32 = 0x0aaa;
 static const u64 kMIPS64_ShadowOffset64 = 1ULL << 37;
 static const u64 kPPC64_ShadowOffset64 = 1ULL << 41;
+static const u64 kRISCV64_ShadowOffset64 = 1ULL << 36;
 static const u64 kSystemZ_ShadowOffset64 = 1ULL << 52;
 static const u64 kSPARC64_ShadowOffset64 = 1ULL << 43;  // 0x800
 static const u64 kFreeBSD_ShadowOffset32 = 1ULL << 30;  // 0x4000
@@ -210,6 +211,8 @@ static const u64 kMyriadCacheBitMask32 = 0x4000ULL;
 #define SHADOW_OFFSET kAArch64_ShadowOffset64
 #  elif defined(__powerpc64__)
 #define SHADOW_OFFSET kPPC64_ShadowOffset64
+#  elif defined(__riscv) && (__riscv_xlen == 64)
+#define SHADOW_OFFSET kRISCV64_ShadowOffset64
 #  elif defined(__s390x__)
 #define SHADOW_OFFSET kSystemZ_ShadowOffset64
 #  elif SANITIZER_FREEBSD
diff --git a/libsanitizer/sanitizer_common/sanitizer_linux.cpp b/libsanitizer/sa
nitizer_common/sanitizer_linux.cpp
index 11c03e286dc..962df07772e 100644
--- a/libsanitizer/sanitizer_common/sanitizer_linux.cpp
+++ b/libsanitizer/sanitizer_common/sanitizer_linux.cpp
@@ -1048,6 +1048,8 @@ uptr GetMaxVirtualAddress() {
   return (1ULL << (MostSignificantSetBitIndex(GET_CURRENT_FRAME()) + 1)) - 1;
 # elif defined(__mips64)
   return (1ULL << 40) - 1;  // 0x00ffUL;
+# elif defined(__riscv) && (__riscv_xlen == 64)
+  return (1ULL << 39) - 1;  // 0x0080UL;
 # elif defined(__s390x__)
   return (1ULL << 53) - 1;  // 0x001fUL;
 #elif defined(__sparc__)
hifiveu017:1193$

I now get
==2936470==AddressSanitizer CHECK failed:
../../../../gcc-git/libsanitizer/sanitizer_common/sanitizer_allocator_primary64.h:76
"((kSpaceBeg)) == ((address_range.Init(TotalSpaceSize,
PrimaryAllocatorName, kSpaceBeg)))" (0x6000,
0x)

so there is still something missing but I think I am closer.

Jim

Re: [PATCH 2/2] gcc/riscv: Add a mechanism to remove some calls to _riscv_save_0

[Jim Wilson]

On Mon, Oct 21, 2019 at 5:26 AM Andrew Burgess
 wrote:
> Below is a new versions of this patch, I believe that this addresses
> the review comments from the earlier version.  In addition this has
> been tested using Jim's idea of forcing -msave-restore (if the flag is
> not otherwise given) and I now see no test failures for newlib and
> linux toolchains.
>
> One thing that has been mentioned briefly was adding a flag to control
> just this optimisation, I haven't done that yet, but can if that is
> required - obviously this optimisation doesn't do anything if
> -msave-restore is turned off, so that is always an option.  With no
> test failures I don't know if a separate flag is required.

I can live without the flag to control it, since as you mention
-msave-restore will turn it off.

I'm seeing failures with the new save-restore-4.c testcase for 32-bit
targets.  It works for 64-bit but not for 32-bit, because the
sign-extension it is checking for only happens for 64-bit targets.
The testcase should check for a target of riscv64*-*-* or else hard
code -march=rv64gc etc options.  Either fix seems reasonable.

You fixed some British spellings of optimise to optimize, but there
are two new comments that add two more uses of optimise which is
inconsistent.

I also noticed a "useage" that should be "usage".

Otherwise this looks good to me.  It is OK to check in with the above
minor issues fixed.

Jim

Re: RFC/A: Add a targetm.vectorize.related_mode hook

[Jim Wilson]

On Wed, Oct 23, 2019 at 4:16 AM Richard Biener
 wrote:
> Note I delayed thinking about relaxing the single-vector-size
> constraint in the vectorizer until after we're SLP only because
> that looked more easily done there.  I also remember patches
> relaxing this a bit from RISCV folks.

Probably not from us RISC-V folks.  I don't know of anyone looking at
RISC-V vector support in gcc other than me, and I've only had time for
some initial exploration.

Jim

Re: [PATCH v2 1/2] RISC-V: Add shorten_memrefs pass

[Jim Wilson]

On Fri, Oct 25, 2019 at 10:40 AM Craig Blackmore
 wrote:
> This patch aims to allow more load/store instructions to be compressed by
> replacing a load/store of 'base register + large offset' with a new load/store
> of 'new base + small offset'. If the new base gets stored in a compressed
> register, then the new load/store can be compressed. Since there is an 
> overhead
> in creating the new base, this change is only attempted when 'base register' 
> is
> referenced in at least 4 load/stores in a basic block.
>
> The optimization is implemented in a new RISC-V specific pass called
> shorten_memrefs which is enabled for RVC targets. It has been developed for 
> the
> 32-bit lw/sw instructions but could also be extended to 64-bit ld/sd in 
> future.

The fact that this needs 4 load/stores in a block with the same base
address means it won't trigger very often.  But it does seem to work.
I see about a 0.05% code size reduction for a rv32gc newlib nano
libc.a.  There might be other codes that benefit more.

I'm concerned about the number of RISC-V specific optimization passes
people are writing.  I've seen at least 3 so far.  This one is at
least small and simple enough to understand that it doesn't look like
it will cause maintenance problems.

The config.gcc change conflicts with the save-restore optimization
pass that Andrew Burgess added, but that is a trivial fix.

The code only works for 32-bit load/stores.  rv64 has compressed
64-bit load/stores, and the F and D extensions have float and double
compressed loads and stores.  The patch would be more useful if it
handled all of these.

The patch doesn't check the other operand, it only looks at the memory
operand.  This results in some cases where the code rewrites
instructions that can never be compressed.  For instance, given
void
store1z (int *array)
{
  array[200] = 0;
  array[201] = 0;
  array[202] = 0;
  array[203] = 0;
}
the patch increases code size by 4 bytes because it rewrites the
stores, but we don't have compressed store 0, and x0 is not a
compressed reg, so there is no point in rewriting the stores.  I can
also create examples that show a size increase with loads but it is a
little trickier.
extern int sub1 (int, int, int, int, int, int, int);
int
load1a (int a0, int a1, int a2, int a3, int a4, int *array)
{
  int a = 0;
  a += array[200];
  a += array[201];
  a += array[202];
  a += array[203];
  return sub1 (a0, a1, a2, a3, a4, 0, a);
}
The register allocator will pass a0-a4 directly through from args to
the call, leaving only a5-a7 for the load base address and dest, and
only one of those regs is a compressed reg, but we need two, so these
loads can't be compressed.  The code still gets rewritten, resulting
in a size increase for the extra add.  Not sure if you can do anything
about that though, since you are doing this before register
allocation.

It isn't clear that the change riscv_address_cost is for.  That should
be commented.

I'd suggest parens in the riscv_compressed_lw_offset_p return
statement, that makes it easier to align the code correctly (in emacs
at least).

The RISCV_MAX_COMPRESSED_LW_OFFSET should be moved down to the "ISA
constants" section of riscv.h, and maybe renamed similar to the other
constants.

There is a REG_P check in get_si_mem_reg.  That should probably handle
SUBREGs too.

A testcase to verify the patch would be nice.  I have one I wrote for
testing that shows some tests that work, some tests that don't work,
and some that work but maybe shouldn't.

I vaguely remember Micheal Meissner talking about doing something
similar for PPC in a GNU Cauldron maybe 2 years ago.  But I don't know
if he tried, or how far he got if he did try.  It might be useful to
ask him about that work.

Otherwise this looks OK to me.

Jim

Re: [PATCH v2 1/2] RISC-V: Add shorten_memrefs pass

[Jim Wilson]

FYI the testcase I'm using to test the patch.  Some of the functions
get smaller, some of them get bigger, and some don't change in size
but should when compiled for an rv64 target.

Jim
void
store1z (int *array)
{
  array[200] = 0;
  array[201] = 0;
  array[202] = 0;
  array[203] = 0;
}

void
store2z (long long *array)
{
  array[200] = 0;
  array[201] = 0;
  array[202] = 0;
  array[203] = 0;
}

void
store1a (int *array, int a)
{
  array[200] = a;
  array[201] = a;
  array[202] = a;
  array[203] = a;
}

void
store2a (long long *array, long long a)
{
  array[200] = a;
  array[201] = a;
  array[202] = a;
  array[203] = a;
}

int
load1r (int *array)
{
  int a = 0;
  a += array[200];
  a += array[201];
  a += array[202];
  a += array[203];
  return a;
}

long long
load2r (long long *array)
{
  int a = 0;
  a += array[200];
  a += array[201];
  a += array[202];
  a += array[203];
  return a;
}

extern int sub1 (int, int, int, int, int, int, int);

int
load1a (int a0, int a1, int a2, int a3, int a4, int *array)
{
  int a = 0;
  a += array[200];
  a += array[201];
  a += array[202];
  a += array[203];
  return sub1 (a0, a1, a2, a3, a4, 0, a);
}

extern long long sub2 (long long, long long, long long, long long, long long,
		   long long, long long);

long long
load2a (long long a0, long long a1, long long a2, long long a3, long long a4,
	long long *array)
{
  int a = 0;
  a += array[200];
  a += array[201];
  a += array[202];
  a += array[203];
  return sub2 (a0, a1, a2, a3, a4, 0, a);
}

Re: [PATCH v2 2/2] sched-deps.c: Avoid replacing address if it increases address cost

[Jim Wilson]

On Fri, Oct 25, 2019 at 10:40 AM Craig Blackmore
 wrote:
> The sched2 pass undoes some of the addresses generated by the RISC-V
> shorten_memrefs code size optimization (patch 1/2) and consequently increases
> code size. This patch prevents sched-deps.c from changing an address if it is
> expected to increase address cost.

This should be rewritten as a target hook, and then we can define the
hook to do what we want for RISC-V.  It isn't OK to make this change
for other targets without testing them.  If the default hook does
nothing, then other targets won't be affected.

Jim

[PATCH] Allow libcalls for complex memcpy when optimizing for size.

[Jim Wilson]

The RISC-V backend wants to use a libcall when optimizing for size if
more than 6 instructions are needed.  Emit_move_complex asks for no
libcalls.  This case requires 8 insns for rv64 and 16 insns for rv32,
so we get fallback code that emits a loop.  Commit_one_edge_insertion
doesn't allow code inserted for a phi node on an edge to end with a
branch, and so this triggers an assertion.  This problem goes away if
we allow libcalls when optimizing for size, which gives the code the
RISC-V backend wants, and avoids triggering the assert.

gcc/
PR middle-end/92263
* expr.c (emit_move_complex): Only use BLOCK_OP_NO_LIBCALL when
optimize_insn_for_speed_p is true.

gcc/testsuite/
PR middle-end/92263
* gcc.dg/pr92263.c: New.
---
 gcc/expr.c |  6 --
 gcc/testsuite/gcc.dg/pr92263.c | 28 
 2 files changed, 32 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr92263.c

diff --git a/gcc/expr.c b/gcc/expr.c
index 476c6865f20..0c2e03dd32d 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -3571,11 +3571,13 @@ emit_move_complex (machine_mode mode, rtx x, rtx y)
   rtx_insn *ret;
 
   /* For memory to memory moves, optimal behavior can be had with the
-existing block move logic.  */
+existing block move logic.  But use normal expansion if optimizing
+for size.  */
   if (MEM_P (x) && MEM_P (y))
{
  emit_block_move (x, y, gen_int_mode (GET_MODE_SIZE (mode), Pmode),
-  BLOCK_OP_NO_LIBCALL);
+  (optimize_insn_for_speed_p()
+   ? BLOCK_OP_NO_LIBCALL : BLOCK_OP_NORMAL));
  return get_last_insn ();
}
 
diff --git a/gcc/testsuite/gcc.dg/pr92263.c b/gcc/testsuite/gcc.dg/pr92263.c
new file mode 100644
index 000..a79dfd1e351
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr92263.c
@@ -0,0 +1,28 @@
+/* { dg-do compile } */
+/* { dg-options "-fno-tree-dce -fno-tree-forwprop -Os -ffloat-store" } */
+
+extern long double cabsl (_Complex long double);
+
+typedef struct {
+  int nsant, nvqd;
+  _Complex long double *vqd;
+} vsorc_t;
+vsorc_t vsorc;
+
+void foo(int next_job, int ain_num, int iped, long t) {
+  long double zpnorm;
+
+  while (!next_job)
+if (ain_num)
+{
+  if (iped == 1)
+zpnorm = 0.0;
+  int indx = vsorc.nvqd-1;
+  vsorc.vqd[indx] = t*1.0fj;
+  if (cabsl(vsorc.vqd[indx]) < 1.e-20)
+vsorc.vqd[indx] = 0.0fj;
+  zpnorm = t;
+  if (zpnorm > 0.0)
+iped = vsorc.nsant;
+}
+}
-- 
2.17.1

Re: [PATCH] Allow libcalls for complex memcpy when optimizing for size.

[Jim Wilson]

On Thu, Oct 31, 2019 at 4:41 PM Jim Wilson  wrote:
> gcc/
> PR middle-end/92263
> * expr.c (emit_move_complex): Only use BLOCK_OP_NO_LIBCALL when
> optimize_insn_for_speed_p is true.
>
> gcc/testsuite/
> PR middle-end/92263
> * gcc.dg/pr92263.c: New.

Tested with rv32-newlib and rv64-linux cross compiler builds and make
checks.  There were no regressions.  The new testcase fails for both
rv32 and rv64 without the patch, and works for both with the patch.

Jim

[PATCH] RISC-V: Build soft-float divide routines for -mno-fdiv.

[Jim Wilson]

Using -mno-fdiv gives linker errors unless we build the missing divide
routines in libgcc always.  There is at least one university project
designing RISC-V parts without FP divide that wants to use the option.

Tested by hand with single-float and double-float builds to verify that
the -mno-fdiv option works with the patch.  Also tested with rv32-newlib
and rv64-linux cross builds and make check, with no regressions.

Committed.

Jim

libgcc/
* config/riscv/t-softfp32 (softfp_extra): Add FP divide routines
---
 libgcc/config/riscv/t-softfp32 | 17 +
 1 file changed, 17 insertions(+)

diff --git a/libgcc/config/riscv/t-softfp32 b/libgcc/config/riscv/t-softfp32
index 1bd51e803d1..59be1df827e 100644
--- a/libgcc/config/riscv/t-softfp32
+++ b/libgcc/config/riscv/t-softfp32
@@ -12,7 +12,11 @@ softfp_float_modes := tf
 softfp_extensions := sftf dftf
 softfp_truncations := tfsf tfdf
 
+# Enable divide routines to make -mno-fdiv work.
+softfp_extras := divsf3 divdf3
+
 else
+# !ABI_DOUBLE
 
 softfp_float_modes := df tf
 softfp_extensions := sfdf sftf dftf
@@ -20,7 +24,20 @@ softfp_truncations := dfsf tfsf tfdf
 
 ifndef ABI_SINGLE
 softfp_float_modes += sf
+else
+# ABI_SINGLE
+
+# Enable divide routines to make -mno-fdiv work.
+softfp_extras := divsf3
+
 endif
 
 endif
+
+else
+# ABI_QUAD
+
+# Enable divide routines to make -mno-fdiv work.
+softfp_extras := divsf3 divdf3 divtf3
+
 endif
-- 
2.17.1

Re: [PATCH] Enable libsanitizer build on riscv64

[Jim Wilson]

On Mon, Nov 11, 2019 at 3:45 AM Andreas Schwab  wrote:
> Only ubsan is supported so far.  This has been tested on openSUSE
> Tumbleweed, there are no testsuite failures.
>
> * configure.tgt (riscv64-*-linux*): Enable build.

I tried this on my riscv64 Fedora system, and I get a build error.

In file included from
../../../../gcc-git/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cpp:167:
../../../../gcc-git/libsanitizer/sanitizer_common/sanitizer_internal_defs.h:342:72:
error: narrowing conversion of ‘-1’ from ‘int’ to ‘long unsigned int’
[-Wnarrowing]
  342 | typedef char IMPL_PASTE(assertion_failed_##_, line)[2*(int)(pred)-1]
  |^
...
../../../../gcc-git/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cpp:1136:1:
note: in expansion of macro ‘CHECK_SIZE_AND_OFFSET’
 1136 | CHECK_SIZE_AND_OFFSET(ipc_perm, mode);
  | ^

It appears that the problem is that my system defines the icp_perm
mode field as __mode_t whereas the sanitizer assumes it will be
unsigned short, followed by an unsigned short pad field.  I haven't
looked at the full history yet, but there were apparently similar
problems with the aarch64 port, so maybe we need some special code for
riscv to make this work?  I don't know why it worked for you.  Maybe a
different glibc or kernel version?

Incidentally, the
libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h file
has an obvious problem in the struct __sanitizer_ipc_perm definition,
because it has an ifdef for __aarch64__ inside an ifdef for __sparc__,
and there is no way the __aarch64__ test can ever succeed there.
There is a second __aarch64__ test a little farther down that looks
OK.  Maybe a patch merge error?

Jim

Re: [PATCH] Enable libsanitizer build on riscv64

[Jim Wilson]

On Mon, Nov 11, 2019 at 3:45 AM Andreas Schwab  wrote:
> Only ubsan is supported so far.  This has been tested on openSUSE
> Tumbleweed, there are no testsuite failures.
>
> * configure.tgt (riscv64-*-linux*): Enable build.

With a workaround for the ipc_perm/mode issue, I can reproduce the
same result on my Fedora rawhide machine.  This patch is OK.

Jim

Re: [PATCH] [PING] Asan changes for RISC-V.

[Jim Wilson]

On Wed, Oct 28, 2020 at 4:59 PM Jim Wilson  wrote:

> We have only riscv64 asan support, there is no riscv32 support as yet.  So
> I
> need to be able to conditionally enable asan support for the riscv
> target.  I
> implemented this by returning zero from the asan_shadow_offset function.
> This
> requires a change to toplev.c and docs in target.def.
>
> The asan support works on a 5.5 kernel, but does not work on a 4.15 kernel.
> The problem is that the asan high memory region is a small wedge below
> 0x40.  The new kernel puts shared libraries at 0x3f and
> going
> down which works.  But the old kernel puts shared libraries at 0x20
> and going up which does not work, as it isn't in any recognized memory
> region.  This might be fixable with more asan work, but we don't really
> need
> support for old kernel versions.
>
> The asan port is curious in that it uses 1<<29 for the shadow offset, but
> all
> other 64-bit targets use a number larger than 1<<32.  But what we have is
> working OK for now.
>
> I did a make check RUNTESTFLAGS="asan.exp" on Fedora rawhide image running
> on
> qemu and the results look reasonable.
>
> === gcc Summary ===
>
> # of expected passes1905
> # of unexpected failures11
> # of unsupported tests  224
>
> === g++ Summary ===
>
> # of expected passes2002
> # of unexpected failures6
> # of unresolved testcases   1
> # of unsupported tests  175
>
> OK?
>
> Jim
>
> 2020-10-28  Jim Wilson  
>
> gcc/
> * config/riscv/riscv.c (riscv_asan_shadow_offset): New.
> (TARGET_ASAN_SHADOW_OFFSET): New.
> * doc/tm.texi: Regenerated.
> * target.def (asan_shadow_offset); Mention that it can return zero.
> * toplev.c (process_options): Check for and handle zero return from
> targetm.asan_shadow_offset call.
>
> Co-Authored-By: cooper.joshua 
> ---
>  gcc/config/riscv/riscv.c | 16 
>  gcc/doc/tm.texi  |  3 ++-
>  gcc/target.def   |  3 ++-
>  gcc/toplev.c |  3 ++-
>  4 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
> index 989a9f15250..6909e200de1 100644
> --- a/gcc/config/riscv/riscv.c
> +++ b/gcc/config/riscv/riscv.c
> @@ -5299,6 +5299,19 @@ riscv_gpr_save_operation_p (rtx op)
>return true;
>  }
>
> +/* Implement TARGET_ASAN_SHADOW_OFFSET.  */
> +
> +static unsigned HOST_WIDE_INT
> +riscv_asan_shadow_offset (void)
> +{
> +  /* We only have libsanitizer support for RV64 at present.
> +
> + This number must match kRiscv*_ShadowOffset* in the file
> + libsanitizer/asan/asan_mapping.h which is currently 1<<29 for rv64,
> + even though 1<<36 makes more sense.  */
> +  return TARGET_64BIT ? (HOST_WIDE_INT_1 << 29) : 0;
> +}
> +
>  /* Initialize the GCC target structure.  */
>  #undef TARGET_ASM_ALIGNED_HI_OP
>  #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
> @@ -5482,6 +5495,9 @@ riscv_gpr_save_operation_p (rtx op)
>  #undef TARGET_NEW_ADDRESS_PROFITABLE_P
>  #define TARGET_NEW_ADDRESS_PROFITABLE_P riscv_new_address_profitable_p
>
> +#undef TARGET_ASAN_SHADOW_OFFSET
> +#define TARGET_ASAN_SHADOW_OFFSET riscv_asan_shadow_offset
> +
>  struct gcc_target targetm = TARGET_INITIALIZER;
>
>  #include "gt-riscv.h"
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index 24c37f655c8..39c596b647a 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -12078,7 +12078,8 @@ is zero, which disables this optimization.
>  @deftypefn {Target Hook} {unsigned HOST_WIDE_INT}
> TARGET_ASAN_SHADOW_OFFSET (void)
>  Return the offset bitwise ored into shifted address to get corresponding
>  Address Sanitizer shadow memory address.  NULL if Address Sanitizer is not
> -supported by the target.
> +supported by the target.  May return 0 if Address Sanitizer is not
> supported
> +by a subtarget.
>  @end deftypefn
>
>  @deftypefn {Target Hook} {unsigned HOST_WIDE_INT} TARGET_MEMMODEL_CHECK
> (unsigned HOST_WIDE_INT @var{val})
> diff --git a/gcc/target.def b/gcc/target.def
> index ed2da154e30..268b56b6ebd 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -4452,7 +4452,8 @@ DEFHOOK
>  (asan_shadow_offset,
>   "Return the offset bitwise ored into shifted address to get
> corresponding\n\
>  Address Sanitizer shadow memory address.  NULL if Address Sanitizer is
> not\n\
> -supported by the target.",
> +supported by the target.  May return 0 if Address Sanitizer is not
> supp

Re: [PATCH v2] Replace dep_list_size with dep_list_costs for better scheduling

[Jim Wilson]

On Thu, Nov 5, 2020 at 6:03 PM Jojo R  wrote:

> gcc/
> * haifa-sched.c (dep_list_costs): New.
> (rank_for_schedule): Use dep_list_costs.
>

When you post a patch, you should explain what the patch is doing and why
this is better than the code that was there before.  It is helpful if you
can show results that demonstrate that it is better, e.g. give a small
example and show some scheduler or assembly output to show what it does.

You should also consider that when you modify target independent code then
you are affecting every target.  This change may work well for your target,
but does it also work for x86, arm, ppc, etc?  This probably requires some
testing to see if it works for other targets.  If not, then maybe it needs
to be conditional on a target hook.

The patch does seem to make some sense though.  When choosing the
instruction that has the most dependent instructions to schedule next, you
want to ignore the ones that have a 0 cost dependency due to a bypass.

Jim

Re: [PATCH v2] Add bypass_p cost check in flag_sched_last_insn_heuristic

[Jim Wilson]

On Thu, Nov 5, 2020 at 6:10 PM Jojo R  wrote:

> gcc/
> * haifa-sched.c (rank_for_schedule): Add bypass_p
> cost check in flag_sched_last_insn_heuristic.
>
> + || (INSN_CODE (DEP_PRO (dep1)) >= 0 && bypass_p (DEP_PRO (dep1))
> + && recog_memoized (DEP_CON (dep1)) >= 0
> + && !dep_cost (dep1)))
>

This is using the same idiom at the previous patch.  Do the two patches
depend on each other?  It isn't clear.  Since this idiom is used 3 times
across the 2 patches, maybe it should be a macro or an inline function.

As with the other patch, some explanation would be nice, and some testing
on multiple targets too.

Jim

Re: [PATCH] [PING^2] Asan changes for RISC-V.

[Jim Wilson]

Original message here
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557406.html

This has non-RISC-V changes, so I need a global reviewer to look at it.

Jim

On Wed, Nov 4, 2020 at 12:10 PM Jim Wilson  wrote:

>
>
> On Wed, Oct 28, 2020 at 4:59 PM Jim Wilson  wrote:
>
>> We have only riscv64 asan support, there is no riscv32 support as yet.
>> So I
>> need to be able to conditionally enable asan support for the riscv
>> target.  I
>> implemented this by returning zero from the asan_shadow_offset function.
>> This
>> requires a change to toplev.c and docs in target.def.
>>
>> The asan support works on a 5.5 kernel, but does not work on a 4.15
>> kernel.
>> The problem is that the asan high memory region is a small wedge below
>> 0x40.  The new kernel puts shared libraries at 0x3f and
>> going
>> down which works.  But the old kernel puts shared libraries at
>> 0x20
>> and going up which does not work, as it isn't in any recognized memory
>> region.  This might be fixable with more asan work, but we don't really
>> need
>> support for old kernel versions.
>>
>> The asan port is curious in that it uses 1<<29 for the shadow offset, but
>> all
>> other 64-bit targets use a number larger than 1<<32.  But what we have is
>> working OK for now.
>>
>> I did a make check RUNTESTFLAGS="asan.exp" on Fedora rawhide image
>> running on
>> qemu and the results look reasonable.
>>
>> === gcc Summary ===
>>
>> # of expected passes1905
>> # of unexpected failures11
>> # of unsupported tests      224
>>
>> === g++ Summary ===
>>
>> # of expected passes2002
>> # of unexpected failures6
>> # of unresolved testcases   1
>> # of unsupported tests  175
>>
>> OK?
>>
>> Jim
>>
>> 2020-10-28  Jim Wilson  
>>
>> gcc/
>> * config/riscv/riscv.c (riscv_asan_shadow_offset): New.
>> (TARGET_ASAN_SHADOW_OFFSET): New.
>> * doc/tm.texi: Regenerated.
>> * target.def (asan_shadow_offset); Mention that it can return
>> zero.
>> * toplev.c (process_options): Check for and handle zero return
>> from
>> targetm.asan_shadow_offset call.
>>
>> Co-Authored-By: cooper.joshua 
>> ---
>>  gcc/config/riscv/riscv.c | 16 
>>  gcc/doc/tm.texi  |  3 ++-
>>  gcc/target.def   |  3 ++-
>>  gcc/toplev.c |  3 ++-
>>  4 files changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
>> index 989a9f15250..6909e200de1 100644
>> --- a/gcc/config/riscv/riscv.c
>> +++ b/gcc/config/riscv/riscv.c
>> @@ -5299,6 +5299,19 @@ riscv_gpr_save_operation_p (rtx op)
>>return true;
>>  }
>>
>> +/* Implement TARGET_ASAN_SHADOW_OFFSET.  */
>> +
>> +static unsigned HOST_WIDE_INT
>> +riscv_asan_shadow_offset (void)
>> +{
>> +  /* We only have libsanitizer support for RV64 at present.
>> +
>> + This number must match kRiscv*_ShadowOffset* in the file
>> + libsanitizer/asan/asan_mapping.h which is currently 1<<29 for rv64,
>> + even though 1<<36 makes more sense.  */
>> +  return TARGET_64BIT ? (HOST_WIDE_INT_1 << 29) : 0;
>> +}
>> +
>>  /* Initialize the GCC target structure.  */
>>  #undef TARGET_ASM_ALIGNED_HI_OP
>>  #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
>> @@ -5482,6 +5495,9 @@ riscv_gpr_save_operation_p (rtx op)
>>  #undef TARGET_NEW_ADDRESS_PROFITABLE_P
>>  #define TARGET_NEW_ADDRESS_PROFITABLE_P riscv_new_address_profitable_p
>>
>> +#undef TARGET_ASAN_SHADOW_OFFSET
>> +#define TARGET_ASAN_SHADOW_OFFSET riscv_asan_shadow_offset
>> +
>>  struct gcc_target targetm = TARGET_INITIALIZER;
>>
>>  #include "gt-riscv.h"
>> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
>> index 24c37f655c8..39c596b647a 100644
>> --- a/gcc/doc/tm.texi
>> +++ b/gcc/doc/tm.texi
>> @@ -12078,7 +12078,8 @@ is zero, which disables this optimization.
>>  @deftypefn {Target Hook} {unsigned HOST_WIDE_INT}
>> TARGET_ASAN_SHADOW_OFFSET (void)
>>  Return the offset bitwise ored into shifted address to get corresponding
>>  Address Sanitizer shadow memory address.  NULL if Address Sanitizer is
>> not
>> -supported by the target.
>> +supported by the target.  May return 0 if Address S

Re: [PATCH] match.pd: undistribute (a << s) & C, when C = (M << s) and exact_log2(M - 1)

[Jim Wilson]

On Wed, Nov 11, 2020 at 2:55 AM Jakub Jelinek via Gcc-patches <
gcc-patches@gcc.gnu.org> wrote:

> On Wed, Nov 11, 2020 at 11:43:34AM +0100, Philipp Tomsich wrote:
> > The patch addresses this by disallowing that rule, if an exact
> power-of-2 is
> > seen as C1.  The reason why I would prefer to have this canonicalised the
> > same way the (X & C1) * C2 is canonicalised, is that cleaning this up
> during
> > combine is more difficult on some architectures that require multiple
> insns
> > to represent the shifted constant (i.e. C1 << C2).
>
> As I said, it is better to decide which one is better before or during
> expansion based on target costs, sure, combine can't catch everything.
>

it could be fixed in combine if we allowed 4 instructions to split into 3.
We allow combinations of 4 insns, but we only allow splits into 2 insns as
far as I know.

Trying 7, 8, 9 -> 10:
7: r80:DI=0x1
8: r81:DI=r80:DI<<0x23
  REG_DEAD r80:DI
  REG_EQUAL 0x8
9: r79:DI=r81:DI-0x8
  REG_DEAD r81:DI
  REG_EQUAL 0x7fff8
   10: r77:DI=r78:DI&r79:DI
  REG_DEAD r79:DI
  REG_DEAD r78:DI
Failed to match this instruction:
(set (reg:DI 77)
(and:DI (reg:DI 78)
(const_int 34359738360 [0x7fff8])))

The AND operation can be implemented with 3 shifts, a left shift to clear
the upper bits, a right shift to clear the lower bits, and then another
left shift to shift it back to position.  We are then left with 4 shifts,
and we can have a combiner pattern to match those 4 shifts and reduce to
2.  But this would require combine.c changes to work.  Unless maybe we
don't split the pattern into 3 insns and accept it as is, but there is risk
that this could result in worse code.

Or alternatively, maybe we could have an ANDDI3 expander which accepts mask
constants like this and emits the 3 shifts directly instead of forcing
the constant to a register.  Then we just need to be able to recognize that
these 3 shifts plus the fourth one can be combined into 2 shifts which
might work already, and if not it should be a simple combiner pattern.
This doesn't help if the same RTL can be created after initial RTL
expansion.

With the draft B extension we do this operation with a single instruction,
but we should get this right for systems without the B extension also.

Jim

Re: [PATCH] RISC-V: Enable ifunc if it was supported in the binutils for linux toolchain.

[Jim Wilson]

On Tue, Nov 10, 2020 at 7:33 PM Nelson Chu  wrote:

> gcc/
> * configure: Regenerated.
> * configure.ac: If ifunc was supported in the binutils for
> linux toolchain, then set enable_gnu_indirect_function to yes.
>

Looks good.  I committed and pushed it.

I see some extra ifunc related testsuite failures, but that is because we
don't have the glibc ifunc patches upstream yet.  It will be important to
get those done next.

Jim

Re: [PATCH] PR target/97682 - Fix to reuse t1 register between call address and epilogue.

[Jim Wilson]

On Mon, Nov 9, 2020 at 11:15 PM Monk Chiang  wrote:

> diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
> index 172c7ca7c98..3bd1993c4c9 100644
> --- a/gcc/config/riscv/riscv.h
> +++ b/gcc/config/riscv/riscv.h
> @@ -342,9 +342,13 @@ extern const char *riscv_default_mtune (int argc,
> const char **argv);
> The epilogue temporary mustn't conflict with the return registers,
> the frame pointer, the EH stack adjustment, or the EH data registers.
> */
>
> -#define RISCV_PROLOGUE_TEMP_REGNUM (GP_TEMP_FIRST + 1)
> +#define RISCV_PROLOGUE_TEMP_REGNUM (GP_TEMP_FIRST)
>  #define RISCV_PROLOGUE_TEMP(MODE) gen_rtx_REG (MODE,
> RISCV_PROLOGUE_TEMP_REGNUM)
>
> +#define RISCV_CALL_ADDRESS_TEMP_REGNUM (GP_TEMP_FIRST + 1)
> +#define RISCV_CALL_ADDRESS_TEMP(MODE) \
> +  gen_rtx_REG (MODE, RISCV_CALL_ADDRESS_TEMP_REGNUM)
>

This looks generally OK, however there is a minor problem that we have code
in riscv_compute_frame_info to save t1 in an interrupt handler register
with a large stack frame, as we know the prologue code will clobber t1 in
this case.  However, with this patch, the prologue now clobbers t0
instead.  So riscv_computer_frame_info needs to be fixed.  I'd suggest
changing the T1_REGNUM to RISCV_PROLOGUE_TEMP_REGNUM to prevent this from
happening again, that is probably my fault.  And the interrupt_save_t1
variable should be renamed, maybe to interupt_save_prologue_temp.

You can see the problem with gcc/testsuite/gcc.target/riscv/interrupt-3.c
if you compile with -O0 and we get
foo:
addi sp,sp,-32
sw t1,28(sp)
sw s0,24(sp)
addi s0,sp,32
li t0,-4096
addi t0,t0,16
add sp,sp,t0
so we are saving t1 and then clobbering t0 with your patch.

Otherwise this looks good.

Jim

Re: [PATCH] Asan changes for RISC-V.

[Jim Wilson]

On Fri, Nov 13, 2020 at 11:12 AM Jeff Law  wrote:

>
> On 10/28/20 5:58 PM, Jim Wilson wrote:
> > We have only riscv64 asan support, there is no riscv32 support as yet.
> So I
> > need to be able to conditionally enable asan support for the riscv
> target.  I
> > implemented this by returning zero from the asan_shadow_offset
> function.  This
> > requires a change to toplev.c and docs in target.def.
> >
>


> I noticed you hadn't committed this change.  Just to be explicit, this
> is OK for the trunk.
>

Thanks committed.  I can self approve the RISC-V parts but not the asan
changes so I wanted a review for that.

Jim

Re: [PATCH] Add a new pattern in 4-insn combine

[Jim Wilson]

On Tue, Nov 10, 2020 at 4:18 PM Jeff Law via Gcc-patches <
gcc-patches@gcc.gnu.org> wrote:

>
> On 11/8/20 7:48 PM, HAO CHEN GUI via Gcc-patches wrote:
> > ChangeLog
> >
> >   * combine.c (combine_validate_cost): Add an argument for newi1pat.
> >   (try_combine): Add a 4-insn combine pattern for optimizing rtx
> >   sign_extend (op:zero_extend, zero_extend).
>
> It'd be nice to see motivating examples.  Depending on their structure,
> we may get better results cleaning things up with match.pd patterns.  We
> already have some which work in this space.
>

I don't have a use case for this specifically, but for the general case of
allowing a 4-insn combine to split into 3 I do have a RISC-V use case for
that in Philipp Tomsich's recent match.pd thread.
https://gcc.gnu.org/pipermail/gcc-patches/2020-November/558765.html

Maybe instead of modifying combine to know about the
sign_extend(zero_extend zero_extend) case you could add a 4->3 splitter to
the rs6000.md file, and then where we call combine_split_insns modify the
code to accept 3 output insns when there were 4 input insns.  That would
fix this rs6000 case, and also allow me to fix my RISC-V case with a
riscv.md splitter.

Jim

Re: [PATCH v2] PR target/97682 - Fix to reuse t1 register between call address and epilogue.

[Jim Wilson]

On Thu, Nov 12, 2020 at 10:56 PM Monk Chiang  wrote:

>   - When expanding the call pattern, choose t1 register be a jump register.
> Epilogue also uses a t1 register to adjust Stack point. The call
> pattern
> and epilogue will initial t1 twice, if both are generated in the same
> function. The call pattern will emit 'la t1,symbol' and 'jalr
> t1'instructions.
> Epilogue also emits 'li t1,4096' and 'addi sp,sp,t1' instructions.
> But li and addi instructions will be placed between la and jalr
> instructions.
> The la instruction will be removed by some optimizations,
> because t1 register define twice, the first define instruction look
> likes duplicate.
>

Thanks.  Committed and pushed.

Jim

Re: [PATCH v1 2/2] RISC-V: Adjust predicates for immediate shift operands

[Jim Wilson]

On Mon, Nov 16, 2020 at 10:57 AM Philipp Tomsich 
wrote:

> In case a negative shift operand makes it through into the backend,
> it will be treated as unsigned and truncated (using a mask) to fit
> into the range 0..31 (for SImode) and 0..63 (for DImode).
>

This is a de-optimization.  This doesn't make any sense.  The ISA manual
clearly states the shift counts are truncated.  Some targets do this with
SHIFT_COUNT_TRUNCATED, but that is known to cause problems, so the RISC-V
port is doing it in the shift expanders.  I believe that other targets do
this too.

Also, note that replacing
  slli a0, a0, 31
with
  li a1, -1;
  sll a0, a0, a1
doesn't change the operation performed.  The shift count is still truncated
to 31, and so you get the exact same result from both code sequences.  All
you have done is make the code bigger and slower which is undesirable.

Also note that the testcase has implementation defined results, so there is
no wrong answer here, and nothing wrong with what the RISC-V port is doing.

+/* { dg-final { scan-assembler "sll" } } */
>

I don't think that this will work as a grep for sll will also match slli.
You would need to add a space or tab or maybe both to the search string to
prevent matches with slli.  Or alternatively use scan-assembler-not "slli"
which will match and fail for both slli and slliw.

Jim

Re: [PATCH v1 1/2] Simplify shifts wider than the bitwidth of types

[Jim Wilson]

On Mon, Nov 16, 2020 at 10:57 AM Philipp Tomsich 
wrote:

> This adds simplify_using_ranges::simplify_lshift_using_ranges to
> detect and rewrite such cases.  If the intersection of meaningful
> shift amounts for the underlying type and the value-range computed
> for the shift-amount (whether an integer constant or a variable) is
> empty, the statement is replaced with the zero-constant of the same
> precision as the result.
>

This has the risk of breaking some user code.  I've seen people write code
for RISC-V knowing that the hardware truncates shift counts, and so not
doing the full calculation to get the right value but just letting the
compiler/hardware calculate it for them via truncation.  Of course this
code has implemented defined result, but there is no reason to break it
unnecessarily.

Jim

Re: [PATCH] RISC-V: Disallow regrenme if the TO register never used before for interrupt functions

[Jim Wilson]

On Mon, Jan 20, 2020 at 12:04 AM Kito Cheng  wrote:
> gcc/ChangeLog
>
> PR target/93304
> * config/riscv/riscv-protos.h (riscv_hard_regno_rename_ok): New.
> * config/riscv/riscv.c (riscv_hard_regno_rename_ok): New.
> * config/riscv/riscv.h (HARD_REGNO_RENAME_OK): Defined.
>
> gcc/testsuite/ChangeLog
>
> PR target/93304
> * gcc.target/riscv/pr93304.c: New test.

OK.

By the way, you don't need blank lines after the PR target/93304.

Jim

Re: [PR 80005] __has_include parsing

[Jim Wilson]

On Mon, Jan 20, 2020 at 5:44 AM Nathan Sidwell  wrote:
> I've pushed this to master, to address 80005
>
> __has_include is funky in that it is macro-like from the POV of #ifdef
> ...

With this patch, __has_include__ no longer works.  There is a use of
this in the RISC-V glibc port.  I see the docs only mention
__has_include, so maybe this has always been a silent bug in the
RISC-V glibc port?  If glibc is broken I can submit a patch.  This
might cause trouble for other people too though.

Given the testcase,
#if __has_include__ ()
#error "yes"
#else
#error "no"
#endif

#if __has_include ()
#error "yes"
#else
#error "no"
#endif

Without this patch I get
rohan:2212$ ./xgcc -B./ -O -S tmp4.c
tmp4.c:2:2: error: #error "yes"
2 | #error "yes"
  |  ^
tmp4.c:8:2: error: #error "yes"
8 | #error "yes"
  |  ^

and with this patch I get
gamma05:2206$ build-install/bin/riscv64-unknown-linux-gnu-gcc -O -S tmp4.c
tmp4.c:1:21: error: missing binary operator before token "("
1 | #if __has_include__ ()
  | ^
tmp4.c:4:2: error: #error "no"
4 | #error "no"
  |  ^
tmp4.c:8:2: error: #error "yes"
8 | #error "yes"
  |  ^

Jim

Re: [PR 80005] __has_include parsing

[Jim Wilson]

On Tue, Jan 21, 2020 at 12:22 AM Jakub Jelinek  wrote:
> I only see one spot where it has been added and then
> 2019-06-06  Florian Weimer  
>
> * sysdeps/unix/sysv/linux/riscv/flush-icache.c: Do not use
> internal GCC preprocessor identifier __has_include__.
> which corrected it to use __has_include.

OK, so upstream glibc is already fixed which is good, but there is at
least one glibc release with the bug, and some trees like
github.com/riscv/riscv-gnu-toolchain that have the broken glibc
sources.  I can deal with that problem though.  We will probably
upgrade glibc when we upgrade gcc anyways.

Jim

Re: [PATCH] RISC-V: Disallow regrenme if the TO register never used before for interrupt functions

[Jim Wilson]

On Mon, Jan 20, 2020 at 6:44 PM Kito Cheng  wrote:
> Thanks, fixed and committed, and it's OK to commit to gcc 8/9 next week?

Yes, that is OK with me.

Jim

Re: [PATCH] riscv: Fix up riscv_rtx_costs for RTL checking (PR target/93333)

[Jim Wilson]

On Tue, Jan 21, 2020 at 12:36 AM Jakub Jelinek  wrote:
> 2020-01-21  Jakub Jelinek  
>
> PR target/9
> * config/riscv/riscv.c (riscv_rtx_costs) : Verify
> the last two operands are CONST_INT_P before using them as such.
>
> * gcc.c-torture/compile/pr9.c: New test.

Yes, this is OK.  I've already tested it with rtl-checking enabled builds.

Jim

[PATCH] RISC-V: Fix rtl checking enabled failure with -msave-restore.

[Jim Wilson]

Found with an rtl checking enabled build and check.  This triggered failures
in the gcc.target/riscv/save-restore* tests.  We are using XINT to access an
XWINT value; INTVAL is the preferred solution.

Since existing tests trigger it we don't need a new one.  Tested with
riscv32-elf and riscv64-linux cross builds and checks with no regressions.

Committed.

Jim

gcc/
* config/riscv/riscv-sr.c (riscv_sr_match_prologue): Use INTVAL
instead of XINT.
---
 gcc/config/riscv/riscv-sr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config/riscv/riscv-sr.c b/gcc/config/riscv/riscv-sr.c
index e3180efcbcc..744d0c48c33 100644
--- a/gcc/config/riscv/riscv-sr.c
+++ b/gcc/config/riscv/riscv-sr.c
@@ -115,7 +115,7 @@ riscv_sr_match_prologue (rtx_insn **body)
   && GET_CODE (XVECEXP (PATTERN (insn), 0, 0)) == UNSPEC_VOLATILE
   && (GET_CODE (XVECEXP (XVECEXP (PATTERN (insn), 0, 0), 0, 0))
  == CONST_INT)
-  && XINT (XVECEXP (XVECEXP (PATTERN (insn), 0, 0), 0, 0), 0) == 2)
+  && INTVAL (XVECEXP (XVECEXP (PATTERN (insn), 0, 0), 0, 0)) == 2)
 return insn;
 
   return NULL;
-- 
2.17.1

[PATCH] RISC-V: Fix combined tree builds.

[Jim Wilson]

The RISC-V toolchain doesn't support leb128 because of linker relaxation
to reduce code size.  This prevents us from computing the leb128 size of a
value at compile time.  So do a configure time gas feature check regardless
of gas version.

The libiconv configure change comes from the recent config/lib-link.m4
patch.  The gcc configure wasn't rebuilt after this change as this was
intended for library configure files.

Tested with riscv32-elf and arm-eabi combined tree builds.  The riscv build
fails without the patch and works with the patch where HAVE_AS_LEB128 is now
false.  The arm build still works and still has HAVE_AS_LEB128 true.

OK?

Jim

gcc/
PR target/91602
* configure.ac (HAVE_AS_LEB128): Delete gas version number in
gcc_GAS_CHECK_FEATURE call.
* configure: Regenerated.
---
 gcc/configure| 39 +++
 gcc/configure.ac |  5 -
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/gcc/configure b/gcc/configure
index e2c8fc71772..2f57fbf3223 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -974,6 +974,7 @@ with_zstd_include
 with_zstd_lib
 enable_rpath
 with_libiconv_prefix
+with_libiconv_type
 enable_sjlj_exceptions
 with_gcc_major_version_only
 enable_secureplt
@@ -1811,6 +1812,7 @@ Optional Packages:
   --with-gnu-ld   assume the C compiler uses GNU ld default=no
   --with-libiconv-prefix[=DIR]  search for libiconv in DIR/include and DIR/lib
   --without-libiconv-prefix don't search for libiconv in includedir and 
libdir
+  --with-libiconv-type=TYPE type of library to search for 
(auto/static/shared)
   --with-gcc-major-version-only
   use only GCC major number in filesystem paths
   --with-pic  try to use only PIC/non-PIC objects [default=use
@@ -10730,6 +10732,16 @@ if test "${with_libiconv_prefix+set}" = set; then :
 
 fi
 
+
+# Check whether --with-libiconv-type was given.
+if test "${with_libiconv_type+set}" = set; then :
+  withval=$with_libiconv_type;  with_libiconv_type=$withval
+else
+   with_libiconv_type=auto
+fi
+
+  lib_type=`eval echo \$with_libiconv_type`
+
   LIBICONV=
   LTLIBICONV=
   INCICONV=
@@ -10767,13 +10779,13 @@ fi
   found_so=
   found_a=
   if test $use_additional = yes; then
-if test -n "$shlibext" && test -f 
"$additional_libdir/lib$name.$shlibext"; then
+if test -n "$shlibext" && test -f 
"$additional_libdir/lib$name.$shlibext" && test x$lib_type != xstatic; then
   found_dir="$additional_libdir"
   found_so="$additional_libdir/lib$name.$shlibext"
   if test -f "$additional_libdir/lib$name.la"; then
 found_la="$additional_libdir/lib$name.la"
   fi
-else
+elif test x$lib_type != xshared; then
   if test -f "$additional_libdir/lib$name.$libext"; then
 found_dir="$additional_libdir"
 found_a="$additional_libdir/lib$name.$libext"
@@ -10797,13 +10809,13 @@ fi
   case "$x" in
 -L*)
   dir=`echo "X$x" | sed -e 's/^X-L//'`
-  if test -n "$shlibext" && test -f "$dir/lib$name.$shlibext"; 
then
+  if test -n "$shlibext" && test -f "$dir/lib$name.$shlibext" 
&& test x$lib_type != xstatic; then
 found_dir="$dir"
 found_so="$dir/lib$name.$shlibext"
 if test -f "$dir/lib$name.la"; then
   found_la="$dir/lib$name.la"
 fi
-  else
+  elif test x$lib_type != xshared; then
 if test -f "$dir/lib$name.$libext"; then
   found_dir="$dir"
   found_a="$dir/lib$name.$libext"
@@ -11031,8 +11043,13 @@ fi
   done
 fi
   else
-
LIBICONV="${LIBICONV}${LIBICONV:+ }-l$name"
-LTLIBICONV="${LTLIBICONV}${LTLIBICONV:+ }-l$name"
+if x$lib_type = 
xauto || x$lib_type = xshared; then
+  LIBICONV="${LIBICONV}${LIBICONV:+ }-l$name"
+  LTLIBICONV="${LTLIBICONV}${LTLIBICONV:+ }-l$name"
+else
+  LIBICONV="${LIBICONV}${LIBICONV:+ }-l:lib$name.$libext"
+  LTLIBICONV="${LTLIBICONV}${LTLIBICONV:+ }-l:lib$name.$libext"
+fi
   fi
 fi
   fi
@@ -23695,18 +23712,16 @@ _ACEOF
 
 
 # Check if we have .[us]leb128, and support symbol arithmetic with it.
+# Targets with aggressive linker relaxation to reduce code size may not be
+# able to support this regardless of gas version, so we don't pass a gas
+# version to force a configure time gas feature check.  RISC-V is an example.
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for .sleb128 and 
.uleb128" >&5
 $as_echo_n "check

Re: [PATCH] RISC-V: Fix combined tree builds.

[Jim Wilson]

ping

Jim

On Thu, Jan 30, 2020 at 2:36 PM Jim Wilson  wrote:
>
> The RISC-V toolchain doesn't support leb128 because of linker relaxation
> to reduce code size.  This prevents us from computing the leb128 size of a
> value at compile time.  So do a configure time gas feature check regardless
> of gas version.
>
> The libiconv configure change comes from the recent config/lib-link.m4
> patch.  The gcc configure wasn't rebuilt after this change as this was
> intended for library configure files.
>
> Tested with riscv32-elf and arm-eabi combined tree builds.  The riscv build
> fails without the patch and works with the patch where HAVE_AS_LEB128 is now
> false.  The arm build still works and still has HAVE_AS_LEB128 true.
>
> OK?
>
> Jim
>
> gcc/
> PR target/91602
> * configure.ac (HAVE_AS_LEB128): Delete gas version number in
> gcc_GAS_CHECK_FEATURE call.
> * configure: Regenerated.
> ---
>  gcc/configure| 39 +++
>  gcc/configure.ac |  5 -
>  2 files changed, 31 insertions(+), 13 deletions(-)
>
> diff --git a/gcc/configure b/gcc/configure
> index e2c8fc71772..2f57fbf3223 100755
> --- a/gcc/configure
> +++ b/gcc/configure
> @@ -974,6 +974,7 @@ with_zstd_include
>  with_zstd_lib
>  enable_rpath
>  with_libiconv_prefix
> +with_libiconv_type
>  enable_sjlj_exceptions
>  with_gcc_major_version_only
>  enable_secureplt
> @@ -1811,6 +1812,7 @@ Optional Packages:
>--with-gnu-ld   assume the C compiler uses GNU ld default=no
>--with-libiconv-prefix[=DIR]  search for libiconv in DIR/include and 
> DIR/lib
>--without-libiconv-prefix don't search for libiconv in includedir and 
> libdir
> +  --with-libiconv-type=TYPE type of library to search for 
> (auto/static/shared)
>--with-gcc-major-version-only
>use only GCC major number in filesystem paths
>--with-pic  try to use only PIC/non-PIC objects [default=use
> @@ -10730,6 +10732,16 @@ if test "${with_libiconv_prefix+set}" = set; then :
>
>  fi
>
> +
> +# Check whether --with-libiconv-type was given.
> +if test "${with_libiconv_type+set}" = set; then :
> +  withval=$with_libiconv_type;  with_libiconv_type=$withval
> +else
> +   with_libiconv_type=auto
> +fi
> +
> +  lib_type=`eval echo \$with_libiconv_type`
> +
>LIBICONV=
>LTLIBICONV=
>INCICONV=
> @@ -10767,13 +10779,13 @@ fi
>found_so=
>found_a=
>if test $use_additional = yes; then
> -if test -n "$shlibext" && test -f 
> "$additional_libdir/lib$name.$shlibext"; then
> +if test -n "$shlibext" && test -f 
> "$additional_libdir/lib$name.$shlibext" && test x$lib_type != xstatic; then
>found_dir="$additional_libdir"
>found_so="$additional_libdir/lib$name.$shlibext"
>if test -f "$additional_libdir/lib$name.la"; then
>  found_la="$additional_libdir/lib$name.la"
>fi
> -else
> +elif test x$lib_type != xshared; then
>if test -f "$additional_libdir/lib$name.$libext"; then
>  found_dir="$additional_libdir"
>  found_a="$additional_libdir/lib$name.$libext"
> @@ -10797,13 +10809,13 @@ fi
>case "$x" in
>  -L*)
>dir=`echo "X$x" | sed -e 's/^X-L//'`
> -  if test -n "$shlibext" && test -f 
> "$dir/lib$name.$shlibext"; then
> +  if test -n "$shlibext" && test -f 
> "$dir/lib$name.$shlibext" && test x$lib_type != xstatic; then
>  found_dir="$dir"
>  found_so="$dir/lib$name.$shlibext"
>  if test -f "$dir/lib$name.la"; then
>found_la="$dir/lib$name.la"
>  fi
> -  else
> +  elif test x$lib_type != xshared; then
>  if test -f "$dir/lib$name.$libext"; then
>found_dir="$dir"
>found_a="$dir/lib$name.$libext"
> @@ -11031,8 +11043,13 @@ fi
>done
>  fi
>else
> -
> LIBICONV="${LIBICONV}${LIBICONV:+ }-l$name"
> -

[committed] RISC-V: Improve caller-save code generation.

[Jim Wilson]

Avoid paradoxical subregs when caller save.  This reduces stack frame size
due to smaller loads and stores, and more frequent rematerialization.

Tested with cross riscv32-elf and riscv64-linux build and check, with no
regressions.

Committed.

Jim

PR target/93532
* config/riscv/riscv.h (HARD_REGNO_CALLER_SAVE_MODE): Define.
---
 gcc/config/riscv/riscv.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
index 19438e28fe8..567c23380fe 100644
--- a/gcc/config/riscv/riscv.h
+++ b/gcc/config/riscv/riscv.h
@@ -268,6 +268,13 @@ along with GCC; see the file COPYING3.  If not see
   1, 1 \
 }
 
+/* Select a register mode required for caller save of hard regno REGNO.
+   Contrary to what is documented, the default is not the smallest suitable
+   mode but the largest suitable mode for the given (REGNO, NREGS) pair and
+   it quickly creates paradoxical subregs that can be problematic.  */
+#define HARD_REGNO_CALLER_SAVE_MODE(REGNO, NREGS, MODE) \
+  ((MODE) == VOIDmode ? choose_hard_reg_mode (REGNO, NREGS, NULL) : (MODE))
+
 /* Internal macros to classify an ISA register's type.  */
 
 #define GP_REG_FIRST 0
-- 
2.17.1

Re: [PATCH 1/2] Add TARGET_COMPUTE_MULTILIB hook to override multi-lib result.

[Jim Wilson]

On Wed, Jul 21, 2021 at 2:28 AM Kito Cheng  wrote:

> Create a new hook to let target could override the multi-lib result,
> the motivation is RISC-V might have very complicated multi-lib re-use
> rule*, which is hard to maintain and use current multi-lib scripts,
> we even hit the "argument list too long" error when we tried to add more
> multi-lib reuse rule.
>

This looks OK to me, though I would rewrite the docs a bit.

> +DEFHOOK
> +(compute_multilib,
> + "Some target like RISC-V might have complicated multilib reuse rule
> which is\
> +  hard to implemented on current multilib scheme, this hook allow target
> to\
> +  override the result from built-in multilib mechanism.\
> +  @var{switches} is the raw option list with @var{n_switches} items;\
> +  @var{multilib_dir} is the multi-lib result which compute by the
> built-in\
> +  multi-lib mechanism;\
> +  @var{multilib_defaults} is the default options list for multi-lib; \
> +  @var{multilib_select} is the string contain the list of supported
> multi-lib, \
> +  and the option checking list. \
> +  @var{multilib_matches}, @var{multilib_exclusions}, and
> @var{multilib_reuse} \
> +  are corresponding to @var{MULTILIB_MATCHES}, @var{MULTILIB_EXCLUSIONS} \
> +  @var{MULTILIB_REUSE}. \
> +  The default definition does nothing but return @var{multilib_dir}
> directly.",
>

I'd suggest instead

"Some targets like RISC-V might have complicated multilib reuse rules
which\n\
are hard to implement with the current multilib scheme.  This hook allows\n\
targets to override the result from the built-in multilib mechanism.\n\
@var{switches} is the raw option list with @var{n_switches} items;\n\
@var{multilib_dir} is the multi-lib result which is computed by the
built-in\n\
multi-lib mechanism;\n\
@var{multilib_defaults} is the default options list for multi-lib;\n\
@var{multilib_select} is the string containing the list of supported\n\
multi-libs, and the option checking list.\n\
@var{multilib_matches}, @var{multilib_exclusions}, and
@var{multilib_reuse}\n\
are corresponding to @var{MULTILIB_MATCHES}, @var{MULTILIB_EXCLUSIONS},\n\
and @var{MULTILIB_REUSE}.\n\
The default definition does nothing but return @var{multilib_dir} directly."

Jim

Re: [PATCH 2/2] RISC-V: Implement TARGET_COMPUTE_MULTILIB

[Jim Wilson]

On Wed, Jul 21, 2021 at 2:28 AM Kito Cheng  wrote:

> Use TARGET_COMPUTE_MULTILIB to search the multi-lib reuse for
> riscv*-*-elf*,
> according following rules:
>

I find the other_cond support a bit confusing.  Is this for -mcmodel
perhaps?  Why not just say that if so?

match_score:
weigth -> weight

riscv_multi_lib_info_t::parse
Calls riscv_subset_list::parse twice when path == ".", the call inside
the if looks unnecessary.

riscv_multilib_lib_check:
Can't found -> Can't find

riscv_check_other_cond:
might got -> might get

riscv_compute_multilib:
bare-matel -> bare-metal
decition -> decision
dection -> decision

It isn't clear how the loop with the comment "ignore march and mabi option
in cond string" can work.  It looks like it computes other_cond, but
assumes that there is at most one other_cond, and that it is always at the
end of the list since otherwise the length won't be computed correctly.
But it doesn't check these constraints.  Do you have examples showing how
this works?
  And maybe a little better commentary explaining what this loop does to
make it easier to understand.  It doesn't mention that it computes
other_cond for instance.

Jim

Re: [PATCH 2/2] RISC-V: Implement TARGET_COMPUTE_MULTILIB

[Jim Wilson]

On Tue, Aug 31, 2021 at 5:22 PM Jim Wilson  wrote:

> On Wed, Jul 21, 2021 at 2:28 AM Kito Cheng  wrote:
>
>> Use TARGET_COMPUTE_MULTILIB to search the multi-lib reuse for
>> riscv*-*-elf*,
>> according following rules:
>>
>
> I find the other_cond support a bit confusing.  Is this for -mcmodel
> perhaps?  Why not just say that if so?
>
> match_score:
> weigth -> weight
>
> riscv_multi_lib_info_t::parse
> Calls riscv_subset_list::parse twice when path == ".", the call inside
> the if looks unnecessary.
>
> riscv_multilib_lib_check:
> Can't found -> Can't find
>
> riscv_check_other_cond:
> might got -> might get
>
> riscv_compute_multilib:
> bare-matel -> bare-metal
> decition -> decision
> dection -> decision
>
> It isn't clear how the loop with the comment "ignore march and mabi
> option in cond string" can work.  It looks like it computes other_cond,
> but assumes that there is at most one other_cond, and that it is always
> at the end of the list since otherwise the length won't be computed
> correctly.  But it doesn't check these constraints.  Do you have examples
> showing how this works?
>   And maybe a little better commentary explaining what this loop does to
> make it easier to understand.  It doesn't mention that it computes
> other_cond for instance.
>

Otherwise it looks OK to me.

Jim

Re: [PATCH] Fix SFmode subreg of DImode and TImode

[Jim Wilson]

On Tue, Sep 7, 2021 at 12:12 AM Michael Meissner via Gcc-patches <
gcc-patches@gcc.gnu.org> wrote:

> This patch fixes the breakage in the PowerPC due to a recent change in
> SUBREG
> behavior.  While it is arguable that the patch that caused the breakage
> should
> be reverted, this patch should be a bandage to prevent these changes from
> happening again.
>

FYI I'm dealing with the same problem in the RISC-V backend via
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102211

My current proposed solution is a new predicate f_register_operand to be
used in patterns that use the f constraint which disallows the subregs that
can't work in FP registers due to NaN-boxing.  This just borrows a few
lines of code that were deleted from validate_subreg and puts it in a
RISC-V specific predicate function.  It worked for newlib and glibc
builds.  I'm trying bootstrap testing now, but my target is a little slower
than yours so this will take some time.

Jim

Re: [PATCH] configure: Update --help output for --with-multilib-list

[Jim Wilson]

On Fri, Sep 17, 2021 at 4:39 AM Jonathan Wakely via Gcc-patches <
gcc-patches@gcc.gnu.org> wrote:

> The list of architectures that support the option is incomplete.
>
> gcc/ChangeLog:
>
> * configure.ac: Fix --with-multilib-list description.
> * configure: Regenerate.
>
> OK for trunk?
>

Looks like or1k has --with-multilib-list support also.  I'd suggest adding
that to the list also.  Ok with that change..

Jim

Re: [RFC PATCH 0/8] RISC-V: Bit-manipulation extension.

[Jim Wilson]

On Thu, Sep 23, 2021 at 12:57 AM Kito Cheng  wrote:

> Bit manipulation extension[1] is finishing the public review and waiting
> for
> the rest of the ratification process, I believe that will become a ratified
> extension soon, so I think it's time to submit to upstream for review now
> :)
>

We still don't have upstream zbs assembler support.  We have rejected other
patches because they didn't upstream the assembler support first.  We
should be following the same rule here for bitmanip.

Maybe we can ask PLCT to write the missing assembler support?

Jim

Re: [RFC PATCH 0/8] RISC-V: Bit-manipulation extension.

[Jim Wilson]

On Mon, Sep 27, 2021 at 4:20 AM Christoph Muellner <
cmuell...@ventanamicro.com> wrote:

> In case somebody wants to test this patchset, a patchset for Binutils
> is required as well.
> AFAIK here would be the Binutils branch with the required changes:
>
> https://github.com/riscv-collab/riscv-binutils-gdb/tree/riscv-binutils-experiment


This branch only has the zba/zbb/zbc support that is already upstream.
There is nothing useful here.

Jim

Re: [RFC PATCH 0/8] RISC-V: Bit-manipulation extension.

[Jim Wilson]

On Tue, Sep 28, 2021 at 3:05 PM Christoph Muellner <
cmuell...@ventanamicro.com> wrote:

> We talked about this in the T&R meeting on Monday.
> Philipp Tomsich mentioned, that he has a patchset from earlier this
> year, which adds support for Zbs.
> He proposed to rebase it and send it to the list in the next days.
>

And this is hopefully a patch that isn't contaminated by any of the changes
from Claire that we can't use.  That is one of the benefits of asking PLCT
as they never worked with Claire.  But at this point I think that enough
time has passed and enough changes were made to the zbs spec, and
considering that there is really only one good way to implement the zbs
binutils support anyways, I think we are probably safe to use patches from
one of us that did work with Claire.

Jim

Re: [PATCH] RISC-V: Pattern name fix mulm3_highpart -> smulm3_highpart.

[Jim Wilson]

On Mon, Sep 27, 2021 at 4:38 AM Geng Qi via Gcc-patches <
gcc-patches@gcc.gnu.org> wrote:

> gcc/ChangeLog:
> * config/riscv/riscv.md
> (muldi3_highpart): Rename to muldi3_highpart.
> (mulditi3): Emit muldi3_highpart.
> (mulsi3_highpart): Rename to mulsi3_highpart.
> (mulsidi3): Emit mulsi3_highpart.
>

This doesn't build on top of tree sources.  It is missing the
mulv3_highpart change I mentioned in the riscv-gcc review.  Also, I
prefer that the order of the changelog entries match the order of hunks in
the patch, it is easier to review that way.  Otherwise, the patch is OK and
I committed it with minor changes.  Since I changed it, I need to send the
patch I did actually commit.

Jim

[PATCH] RISC-V: Pattern name fix mul*3_highpart -> smul*3_highpart.

[Jim Wilson]

From: Geng Qi 

No known code changes, just fixes an inconsistency that was noticed.

Committed.

Jim

gcc/
* config/riscv/riscv.md (mulv4): Call gen_smul3_highpart.
(mulditi3): Call muldi3_highpart.
(muldi3_highpart): Rename to muldi3_highpart.
(mulsidi3): Call mulsi3_highpart.
(mulsi3_highpart): Rename to mulsi3_highpart.
---
 gcc/config/riscv/riscv.md | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index f88877fd596..98364f00f6d 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -802,7 +802,7 @@ (define_expand "mulv4"
   rtx hp = gen_reg_rtx (mode);
   rtx lp = gen_reg_rtx (mode);
 
-  emit_insn (gen_mul3_highpart (hp, operands[1], operands[2]));
+  emit_insn (gen_smul3_highpart (hp, operands[1], operands[2]));
   emit_insn (gen_mul3 (operands[0], operands[1], operands[2]));
   emit_insn (gen_ashr3 (lp, operands[0],
  GEN_INT (BITS_PER_WORD - 1)));
@@ -899,14 +899,14 @@ (define_expand "mulditi3"
   emit_insn (gen_muldi3 (low, operands[1], operands[2]));
 
   rtx high = gen_reg_rtx (DImode);
-  emit_insn (gen_muldi3_highpart (high, operands[1], operands[2]));
+  emit_insn (gen_muldi3_highpart (high, operands[1], operands[2]));
 
   emit_move_insn (gen_lowpart (DImode, operands[0]), low);
   emit_move_insn (gen_highpart (DImode, operands[0]), high);
   DONE;
 })
 
-(define_insn "muldi3_highpart"
+(define_insn "muldi3_highpart"
   [(set (match_operand:DI0 "register_operand" "=r")
(truncate:DI
  (lshiftrt:TI
@@ -961,13 +961,13 @@ (define_expand "mulsidi3"
 {
   rtx temp = gen_reg_rtx (SImode);
   emit_insn (gen_mulsi3 (temp, operands[1], operands[2]));
-  emit_insn (gen_mulsi3_highpart (riscv_subword (operands[0], true),
+  emit_insn (gen_mulsi3_highpart (riscv_subword (operands[0], true),
 operands[1], operands[2]));
   emit_insn (gen_movsi (riscv_subword (operands[0], false), temp));
   DONE;
 })
 
-(define_insn "mulsi3_highpart"
+(define_insn "mulsi3_highpart"
   [(set (match_operand:SI0 "register_operand" "=r")
(truncate:SI
  (lshiftrt:DI
-- 
2.25.1

[PATCH] RISC-V: Disable use of TLS copy relocs.

[Jim Wilson]

Musl and lld don't support TLS copy relocs, and don't want to add support
for this feature which is unique to RISC-V.  Only GNU ld and glibc support
them.  In the pasbi discussion, people have pointed out various problems
with using them, so we are deprecating them.  There doesn't seem to be an
ABI break from dropping them so this patch modifies gcc to stop creating
them.  I'm using an ifdef for now in case a problem turns up and the code
has to be re-enabled.  The plan is to add an initial to local exec
relaxation as a replacement, though this has not been defined or
implemented yet.

This was tested with native gcc and glibc builds and checks with no
regressions.

Committed.

Jim

gcc/
* config/riscv/riscv.c (riscv_legitimize_tls_address): Ifdef out
use of TLS_MODEL_LOCAL_EXEC when not pic.
---
 gcc/config/riscv/riscv.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index 3e0bedaf145..4ba811126fe 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -1257,9 +1257,12 @@ riscv_legitimize_tls_address (rtx loc)
   rtx dest, tp, tmp;
   enum tls_model model = SYMBOL_REF_TLS_MODEL (loc);
 
+#if 0
+  /* TLS copy relocs are now deprecated and should not be used.  */
   /* Since we support TLS copy relocs, non-PIC TLS accesses may all use LE.  */
   if (!flag_pic)
 model = TLS_MODEL_LOCAL_EXEC;
+#endif
 
   switch (model)
 {
-- 
2.17.1

Re: [PATCH v2] REE: PR rtl-optimization/100264: Handle more PARALLEL SET expressions

[Jim Wilson]

On Mon, May 10, 2021 at 5:39 AM Christoph Muellner 
wrote:

> gcc/ChangeLog:
> PR rtl-optimization/100264
> * ree.c (get_sub_rtx): Ignore SET expressions without register
> destinations and remove assertion, as it is not valid anymore
> with this new behaviour.
> (merge_def_and_ext): Eliminate destination check for register
> as such SET expressions can't occur anymore.
> (combine_reaching_defs): Likewise.
>

The revised patch looks OK to me, and passed my testing.  I pushed it.

Jim

[PATCH] RISC-V: Enable riscv attributes by default for all riscv targets.

[Jim Wilson]

These were only enabled for embedded elf originally because that was
the safe option, and linux had no obvious use for them.  But now that
we have new extensions coming like V that affect process state and ABIs,
the attributes are expected to be useful for linux, and may be required
by the psABI.  clang already emits them for all riscv targets.

Tested with a patched open embedded build and boot, and a native
toolchain build.

Committed.

Jim

gcc/
* config.gcc (riscv*-*-*): If --with-riscv-attribute not used,
turn TARGET_RISCV_ATTRIBUTES on for all riscv targets.
---
 gcc/config.gcc | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 92fad8e20ca..6833a6c13d9 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4605,14 +4605,7 @@ case "${target}" in
tm_defines="${tm_defines} TARGET_RISCV_ATTRIBUTE=0"
;;
""|default)
-   case "${target}" in
-   riscv*-*-elf*)
-   tm_defines="${tm_defines} 
TARGET_RISCV_ATTRIBUTE=1"
-   ;;
-   *)
-   tm_defines="${tm_defines} 
TARGET_RISCV_ATTRIBUTE=0"
-   ;;
-   esac
+   tm_defines="${tm_defines} TARGET_RISCV_ATTRIBUTE=1"
;;
*)
echo "--with-riscv-attribute=${with_riscv_attribute} is 
not supported.  The argument must begin with yes, no or default." 1>&2
-- 
2.25.1

Re: RISCV: Add zmmul extension

[Jim Wilson]

On Wed, Oct 27, 2021 at 12:14 AM Kito Cheng  wrote:

> Otherwise it is LGTM, but I'm just surprised it's still 0.1 and not frozen
> yet.
>

We should have binutils support first before we have gcc support.
Otherwise that may lead to binutils errors later when zmmul gets passed
down to binutils.  I didn't see a binutils patch yet.

Jim

Re: [PATCH 1/2] RISC-V: Add arch flags for T-HEAD.

[Jim Wilson]

On Tue, Jul 13, 2021 at 11:06 AM Palmer Dabbelt  wrote:

> Is there are documentation as to what this "theadc" extension is?
>

The best doc I know of is
https://github.com/isrc-cas/c910-llvm
The README is in Chinese, but google translate does a decent job on it.  If
you want more details, you have to read the llvm sources to see exactly
what each instruction does.  They have mentioned that they are working on
English language docs, but I don't know when they will be available.

There are quite a few T-Head specific instructions here.  This patch is
only adding support for a few of them, probably as a trial to see how it
goes before they try to add the rest.

Jim

Re: [PATCH 10/10] RISC-V: Provide programmatic implementation of CAS [PR 100266]

[Jim Wilson]

On Mon, Apr 26, 2021 at 5:46 AM Christoph Muellner 
wrote:

> The existing CAS implementation uses an INSN definition, which provides
> the core LR/SC sequence. Additionally to that, there is a follow-up code,
> that evaluates the results and calculates the return values.
> This has two drawbacks: a) an extension to sub-word CAS implementations
> is not possible (even if, then it would be unmaintainable), and b) the
> implementation is hard to maintain/improve.
> This patch provides a programmatic implementation of CAS, similar
> like many other architectures are having one.


I noticed that when the address isn't already valid for lr/sc then we end
up with extra instructions to fix the address.  For instance, using
gcc/testsuite/gcc.dg/atomic-compare-exchange-3.c, I get for the lr/sc loop
.L2:
addi a5,a3,%lo(v)
lr.w a1, 0(a5)
bne a1,a2,.L7
addi a1,a3,%lo(v)
sc.w a5, a0, 0(a1)
sext.w a5,a5
bne a5,zero,.L2
and note that there are two addi %lo instructions.  The current code gives
 addi a4,a4,%lo(v)
1: lr.w a2,0(a4); bne a2,a5,1f; sc.w a6,a0,0(a4); bnez a6,1b; 1:
which is better, as the address is fixed before the lr/sc loop.

The sext is fixed by the REE patch, or by directly generating the
sign-extending sc.w so that isn't an issue here.

Jim

Re: [PATCH] [RISCV] Add Pattern for builtin overflow

[Jim Wilson]

On Tue, Apr 27, 2021 at 12:45 AM Andrew Waterman  wrote:

> > signed addition (SImode with RV64):
> > add t0, t1, t2
> > sext.w  t3, t0
> > bne t0, t3, overflow
>
> The following version has the same instruction count but offers more ILP:
>
>   add t0, t1, t2
>   addw t3, t1, t2
>   bne t0, t3, overflow
>

This is a good suggestion, but in the interests of making forward progress
here, I'd like to accept the patch and then file these as bugzillas as ways
to further improve the patch.

> > unsigned addition (SImode with RV64):
> > sext.w  t3, t1
> > addwt0, t1, t2
> > bltut0, t3, overflow
>
> I think you can do this in two instructions, similar to the previous
> pattern:
>
>   addw t0, t1, t2
>   bltu t0, t1, overflow
>

Likewise.

> > signed subtraction (SImode with RV64):
> > sub t0, t1, t2
> > sext.w  t3, t0
> > bne t0, t3, overflow
>
> See analogous addition comment.
>

Likewise.

>
> > unsigned subtraction (SImode with RV64):
> > sext.w  t3, t1
> > subwt0, t1, t2
> > bltut0, t3, overflow
>
> See analogous addition comment.
>

Likewise.

> > unsigned multiplication (SImode with RV64):
> > sllit0,t0,32
> > sllit1,t1,32
> > srlit0,t0,32
> > srlit1,t1,32
> > mul t0,t0,t1
> > srait5,t0,32
> > bne t5, 0, overflow
>
> I think you can eliminate the first two right shifts by replacing mul
> with mulhu... something like:
>
>   slli rx, rx, 32
>   slli ry, ry, 32
>   mulhu rz, rx, ry
>   srli rt, rz, 32
>   bnez rt, overflow
>

Likewise, except this should be a separate bugzilla.

Jim

Re: [PATCH 00/10] [RISC-V] Atomics improvements [PR100265/PR100266]

[Jim Wilson]

On Mon, Apr 26, 2021 at 5:45 AM Christoph Muellner 
wrote:

> This series provides a cleanup of the current atomics implementation
> of RISC-V:
>

This looks OK to me, other than the issue with address instructions emitted
inside the lr/sc loop.  That could be fixed with a follow up patch though.

amoswap is probably faster than a fence followed by a store, but they
aren't exactly the same operation, since I think the amoswap only provides
consistency for the target address whereas the fence would provide
consistency for all addresses.  It does look odd that we were using amoswap
here.  The inconsistency with atomic_load might be a problem.  I'm OK with
dropping this.

I'm not sure if we are implementing the __sync_* builtins properly.  Most
of them are defined as full barriers.  With our weak memory model RVWMO,
that implies that we need a fence along with an amo instruction.  However,
this is broken without your patch, and not made any worse by your patch, so
it isn't an issue with your patch.  Just a FYI.  See for instance the
discussion in PR 65697 and the patch to fix this for aarch64.
https://gcc.gnu.org/git/?p=gcc.git;a=blobdiff;f=gcc/config/aarch64/aarch64.c;h=93bea074d6831b2aea2c0a40dacddc9ad20788c7;hp=648a548e0e06d2968fb74e4b588c368db060ad74;hb=f70fb3b635f9618c6d2ee3848ba836914f7951c2;hpb=fc65eccabc6d6e881ff5efcd674aa3791cf8cee6

Jim

Re: [PATCH] [RISCV] Add Pattern for builtin overflow

[Jim Wilson]

On Wed, Apr 28, 2021 at 4:04 PM Andrew Waterman  wrote:

> > This is a good suggestion, but in the interests of making forward
> progress here, I'd like to accept the patch and then file these as
> bugzillas as ways to further improve the patch.
>
> Agreed, these potential improvements are definitely not blockers.
>

Turns out Levy had time to work on the patch after all, and submitted a
fourth version with your improvements.

Jim

Re: [PATCH] [RISCV] Add Pattern for builtin overflow

[Jim Wilson]

On Wed, Apr 28, 2021 at 10:43 PM Levy Hsu  wrote:

> From: LevyHsu 
>
> Added implementation for builtin overflow detection, new patterns are
> listed below.
>

This looks OK.  You are missing a ChangeLog entry.  I added one.  I had to
fix some whitespace and formatting issues.  Open parens should line up in
the RTL patterns.  There should be no lines that start with 8 spaces, use a
tab instead.  There should be no lines with only whitespace on them.  You
didn't indent open curly braces in some places.  You missed indenting the
first line in a pattern.  You had a blank line at the start of an output
template.  All simple stuff that I fixed.  Then committed the patch.

Jim

Re: About implementation of the Negative property of options.

[Jim Wilson]

On Wed, Apr 28, 2021 at 1:11 PM Joseph Myers 
wrote:

> Could you please explain the bug at the *user-visible* level?  That is,
> the particular options passed to the compiler, how those options behave,
> and how you think they should behave instead.


I added this to the riscv.opt file to create some new options for
demonstration purposes.  The same changes probably work for other targets
too.

mfoo1=
Target Joined Var(riscv_isa_foo)

mfoo2=
Target Joined Var(riscv_isa_foo) RejectNegative

mfoo3=
Target Joined Var(riscv_isa_foo) Negative(mfoo3=)

mfoo4=
Target Joined Var(riscv_isa_foo) Negative(mfoo5=)

mfoo5=
Target Joined Var(riscv_isa_foo) Negative(mfoo4=)

mfoo6=
Target Joined Var(riscv_isa_foo) Negative(mfoo6=) RejectNegative

mfoo7=
Target Joined Var(riscv_isa_foo) Negative(mfoo8=) RejectNegative

mfoo8=
Target Joined Var(riscv_isa_foo) Negative(mfoo7=) RejectNegative

TargetVariable
int riscv_isa_foo = 0

Then I ran some commands to look at the cc1 option list.

rohan:2754$ ./xgcc -B./ -mfoo1=10 -mfoo1=20 tmp.c -v -S |& grep cc1
 ./cc1 -quiet -v -imultilib rv64imafdc/lp64d -iprefix
/home/jimw/FOSS/GCC/X-9-riscv64-elf/gcc/../lib/gcc/riscv64-unknown-elf/9.3.1/
-isystem ./include -isystem ./include-fixed tmp.c -quiet -dumpbase tmp.c
-mfoo1=10 -mfoo1=20 -march=rv64gc -mabi=lp64d -auxbase tmp -version -o tmp.s
rohan:2755$ ./xgcc -B./ -mfoo2=10 -mfoo2=20 tmp.c -v -S |& grep cc1
 ./cc1 -quiet -v -imultilib rv64imafdc/lp64d -iprefix
/home/jimw/FOSS/GCC/X-9-riscv64-elf/gcc/../lib/gcc/riscv64-unknown-elf/9.3.1/
-isystem ./include -isystem ./include-fixed tmp.c -quiet -dumpbase tmp.c
-mfoo2=10 -mfoo2=20 -march=rv64gc -mabi=lp64d -auxbase tmp -version -o tmp.s
rohan:2756$ ./xgcc -B./ -mfoo3=10 -mfoo3=20 tmp.c -v -S |& grep cc1
 ./cc1 -quiet -v -imultilib rv64imafdc/lp64d -iprefix
/home/jimw/FOSS/GCC/X-9-riscv64-elf/gcc/../lib/gcc/riscv64-unknown-elf/9.3.1/
-isystem ./include -isystem ./include-fixed tmp.c -quiet -dumpbase tmp.c
-mfoo3=10 -mfoo3=20 -march=rv64gc -mabi=lp64d -auxbase tmp -version -o tmp.s
rohan:2757$ ./xgcc -B./ -mfoo4=10 -mfoo4=20 tmp.c -v -S |& grep cc1
 ./cc1 -quiet -v -imultilib rv64imafdc/lp64d -iprefix
/home/jimw/FOSS/GCC/X-9-riscv64-elf/gcc/../lib/gcc/riscv64-unknown-elf/9.3.1/
-isystem ./include -isystem ./include-fixed tmp.c -quiet -dumpbase tmp.c
-mfoo4=10 -mfoo4=20 -march=rv64gc -mabi=lp64d -auxbase tmp -version -o tmp.s
rohan:2758$ ./xgcc -B./ -mfoo5=10 -mfoo5=20 tmp.c -v -S |& grep cc1
 ./cc1 -quiet -v -imultilib rv64imafdc/lp64d -iprefix
/home/jimw/FOSS/GCC/X-9-riscv64-elf/gcc/../lib/gcc/riscv64-unknown-elf/9.3.1/
-isystem ./include -isystem ./include-fixed tmp.c -quiet -dumpbase tmp.c
-mfoo5=10 -mfoo5=20 -march=rv64gc -mabi=lp64d -auxbase tmp -version -o tmp.s
rohan:2759$ ./xgcc -B./ -mfoo6=10 -mfoo6=20 tmp.c -v -S |& grep cc1
 ./cc1 -quiet -v -imultilib rv64imafdc/lp64d -iprefix
/home/jimw/FOSS/GCC/X-9-riscv64-elf/gcc/../lib/gcc/riscv64-unknown-elf/9.3.1/
-isystem ./include -isystem ./include-fixed tmp.c -quiet -dumpbase tmp.c
-mfoo6=20 -march=rv64gc -mabi=lp64d -auxbase tmp -version -o tmp.s
rohan:2760$ ./xgcc -B./ -mfoo7=10 -mfoo7=20 tmp.c -v -S |& grep cc1
 ./cc1 -quiet -v -imultilib rv64imafdc/lp64d -iprefix
/home/jimw/FOSS/GCC/X-9-riscv64-elf/gcc/../lib/gcc/riscv64-unknown-elf/9.3.1/
-isystem ./include -isystem ./include-fixed tmp.c -quiet -dumpbase tmp.c
-mfoo7=10 -mfoo7=20 -march=rv64gc -mabi=lp64d -auxbase tmp -version -o tmp.s
rohan:2761$ ./xgcc -B./ -mfoo7=10 -mfoo8=20 tmp.c -v -S |& grep cc1
 ./cc1 -quiet -v -imultilib rv64imafdc/lp64d -iprefix
/home/jimw/FOSS/GCC/X-9-riscv64-elf/gcc/../lib/gcc/riscv64-unknown-elf/9.3.1/
-isystem ./include -isystem ./include-fixed tmp.c -quiet -dumpbase tmp.c
-mfoo7=10 -mfoo8=20 -march=rv64gc -mabi=lp64d -auxbase tmp -version -o tmp.s
rohan:2762$

Note that only in the -mfoo6= case are the duplicate options removed from
the command line.  So pruning options requires that you have both
RejectNegative and Negative pointing at yourself, which is not what the
documentation says.

The original bug report mentioned problems with picking the right multilib
if you have multiple conflicting options on the command line.  But another
simpler way to show the problem is by mixing 32-bit and 64-bit arches on
the command line.

rohan:2546$ riscv64-unknown-elf-gcc -march=rv32gc -march=rv64gc -mabi=lp64d
tmp.c
/home/jimw/FOSS/install-riscv64/lib/gcc/riscv64-unknown-elf/10.2.0/../../../../riscv64-unknown-elf/bin/ld:
unrecognised emulation mode: elf3264lriscv
Supported emulations: elf64lriscv elf32lriscv
collect2: error: ld returned 1 exit status
rohan:2547$

This is because we have
#define XLEN_SPEC \
  "%{march=rv32*:32}" \
  "%{march=rv64*:64}" \

#define LINK_SPEC "\

-melf" XLEN_SPEC DEFAULT_ENDIAN_SPEC "riscv \

which only works right if contradictory options are pruned.

I tried a gcc-9 tree that I have handy, and got the same result.  Only in
the -mfoo6= case are the contradictory options pruned away, so this does
not appear to be due to a recent

Re: [PATCH] RISC-V: For '-march' and '-mabi' options, add 'Negative' property mentions itself.

[Jim Wilson]

On Wed, Apr 28, 2021 at 1:30 AM Geng Qi via Gcc-patches <
gcc-patches@gcc.gnu.org> wrote:

> gcc/ChangeLog:
> * config/riscv/riscv.opt (march=,mabi=): Negative itself.
>

Thanks.  I committed this.

Do we need to backport to release branches?  This looks like an uncommon
problem, or we would have noticed this before.

Jim

Re: About implementation of the Negative property of options.

[Jim Wilson]

On Fri, Apr 30, 2021 at 1:03 AM gengqi-linux 
wrote:

> Thanks for your replies.
>

I would suggest filing a bug report, and adding useful info from this
thread to the bug report.  Then we can track it.

Jim

Re: [PATCH] doc/options.texi: Fix the discription of 'Negative'.

[Jim Wilson]

On Wed, Apr 28, 2021 at 2:25 AM Geng Qi via Gcc-patches <
gcc-patches@gcc.gnu.org> wrote:

> gcc/ChangeLog:
> * doc/options.texi (Negative): Fix the discription so that it
> matches
> the code implementation of prune_options().
>

This matches my testing as mentioned in another thread, so I am approving
this and committed it, with the missing article added as Martin mentioned.

Jim

Re: [PATCH] RISC-V: Implement __clear_cache via __builtin__clear_cache

[Jim Wilson]

On Thu, Apr 29, 2021 at 10:02 PM Palmer Dabbelt 
wrote:

> This was reported as Bug 94136, which is a year old but was categorized
> as a documentation bug.  I believe that categorization was incorrect:
> having an empty __clear_cache library routine is simply incorrect


It affects almost all targets, and fixing RISC-V does nothing useful to
change that.  x86_64-linux for instance also has an empty __clear_cache
function in libgcc.a.  We can either fix almost all targets, or we can fix
the docs to match reality.  Fixing the docs is easier.  This is why it is
classified as a doc bug.

Fixing riscv is nice.  You can remove the riscv target setting.  But you
can't close it because it is still broken for a lot of other targets.
Unless maybe this trick of defining CLEAR_INSN_CACHE works for other
targets too, in which case maybe we could add a default definition
someplace for targets that don't have their own definition that does what
your RISC-V patch does.

+/* We always have a "clear_cache" insn, which means __builtin__clear_cache
> will
> +   never emit a call to __clear_cache.  */
> +#undef CLEAR_INSN_CACHE
> +#define CLEAR_INSN_CACHE(BEG, END) __builtin__clear_cache((BEG), (END));
>

It is __builtin___clear_cache with 3 underscores before the clear.  With
this change it works for me.

Jim

Re: [PATCH v2 09/10] RISC-V: Provide programmatic implementation of CAS [PR 100266]

[Jim Wilson]

On Wed, May 5, 2021 at 12:37 PM Christoph Muellner 
wrote:

> The existing CAS implementation uses an INSN definition, which provides
> the core LR/SC sequence. Additionally to that, there is a follow-up code,
> that evaluates the results and calculates the return values.
> This has two drawbacks: a) an extension to sub-word CAS implementations
> is not possible (even if, then it would be unmaintainable), and b) the
> implementation is hard to maintain/improve.
> This patch provides a programmatic implementation of CAS, similar
> like many other architectures are having one.
>

A comment that Andrew Waterman made to me today about the safety of this
under various circumstances got me thinking, and I realized that without
the special cas pattern we can get reloads in the middle of the sequence
which would be bad.  Experimenting a bit, I managed to prove it.  This is
using the old version of the patch which I already had handy, but I'm sure
the new version will behave roughly the same way.  Using the testsuite
testcase atomic-compare-exchange-3.c as before, and adding a lot of
-ffixed-X options to simulate high register pressure, with the compiler
command
./xgcc -B./ -O2 -S tmp.c -ffixed-x16 -ffixed-x17 -ffixed-x18 -ffixed-x19
-ffixed-x20 -ffixed-x21 -ffixed-x22 -ffixed-x23 -ffixed-x24 -ffixed-x25
-ffixed-x26 -ffixed-x27 -ffixed-x28 -ffixed-x29 -ffixed-x30 -ffixed-x31
-ffixed-x15 -ffixed-x14 -ffixed-x13 -ffixed-x12 -ffixed-s0 -ffixed-s1
-ffixed-t2 -ffixed-t1 -ffixed-t0
I get for the first lr/sc loop
.L2:
lui a1,%hi(v)
addia0,a1,%lo(v)
lr.w a1, 0(a0)
ld  a0,8(sp)
sw  a1,24(sp)
bne a1,a0,.L39
lui a1,%hi(v)
addia0,a1,%lo(v)
lw  a1,16(sp)
sd  ra,24(sp)
sc.w ra, a1, 0(a0)
sext.w  a1,ra
ld  ra,24(sp)
bne a1,zero,.L2
and note all of the misc load/store instructions added by reload.  I don't
think this is safe or guaranteed to work.  With the cas pattern, any
reloads are guaranteed to be emitted before and/or after the lr/sc loop.
With the separate patterns, there is no way to ensure that we won't get
accidental reloads in the middle of the lr/sc loop.

I think we need to keep the cas pattern.  We can always put C code inside
the output template of the cas pattern if that is helpful.  It can do any
necessary tests and then return an appropriate string for the instructions
we want.

Jim

Re: [PATCH] RISC-V: Generate helpers for cbranch4

[Jim Wilson]

On Wed, May 5, 2021 at 12:23 PM Christoph Muellner 
wrote:

> gcc/
> PR 100266
> * config/rsicv/riscv.c (riscv_block_move_loop): Simplify.
> * config/rsicv/riscv.md (cbranch4): Generate helpers.
>

OK.  Committed.  Though I had to fix the ChangeLog entry.  It was indented
by spaces instead of tabs.  The PR line is missing the component (target).
riscv is misspelled twice as rsicv.  And it doesn't mention the
stack_protect_test change.  The gcc commit hooks complained about most of
this stuff.  It seems fairly good at finding minor ChangeLog issues.

Jim

Re: [PATCH 1/2] REE: PR rtl-optimization/100264: Handle more PARALLEL SET expressions

[Jim Wilson]

On Fri, Apr 30, 2021 at 4:10 PM Christoph Müllner via Gcc-patches <
gcc-patches@gcc.gnu.org> wrote:

> On Sat, May 1, 2021 at 12:48 AM Jeff Law  wrote:
> > On 4/26/2021 5:38 AM, Christoph Muellner via Gcc-patches wrote:
> > > [ree] PR rtl-optimization/100264: Handle more PARALLEL SET expressions
> > >
> > >  PR rtl-optimization/100264
> > >  * ree.c (get_sub_rtx): Ignore SET expressions without register
> > >  destinations.
> > >  (merge_def_and_ext): Eliminate destination check for register
> > >  as such SET expressions can't occur anymore.
> > >  (combine_reaching_defs): Likewise.
> >
> > This is pretty sensible.  Do you have commit privs for GCC?
>

This looks reasonable to me also.  But I tried a build and check with an
rv64gc/lp64d linux toolchain built from riscv-gnu-toolchain and I get two
extra failures in the gfortran testsuite.

/scratch/jimw/fsf-testing/patched/riscv-gcc/gcc/testsuite/gfortran.dg/typebound\
_operator_3.f03:93:21: internal compiler error: in get_sub_rtx, at
ree.c:705^M
0x15664f8 get_sub_rtx^M
../../../patched/riscv-gcc/gcc/ree.c:705^M
0x15672ce merge_def_and_ext^M
../../../patched/riscv-gcc/gcc/ree.c:719^M
0x15672ce combine_reaching_defs^M
../../../patched/riscv-gcc/gcc/ree.c:1020^M
0x1568308 find_and_remove_re^M
../../../patched/riscv-gcc/gcc/ree.c:1319^M
0x1568308 rest_of_handle_ree^M
../../../patched/riscv-gcc/gcc/ree.c:1390^M
0x1568308 execute^M
../../../patched/riscv-gcc/gcc/ree.c:1418^M
Please submit a full bug report,^M
with preprocessed source if appropriate.^M
Please include the complete backtrace with any bug report.^M
See  for instructions.^M
compiler exited with status 1
FAIL: gfortran.dg/typebound_operator_3.f03   -Os  (internal compiler error)
FAIL: gfortran.dg/typebound_operator_3.f03   -Os  (test for excess errors)

Jim

Re: [PATCH, ARM] stop changing signedness in PROMOTE_MODE

[Jim Wilson]

On Wed, Jul 8, 2015 at 3:54 PM, Jeff Law  wrote:
> On 07/07/2015 10:29 AM, Jim Wilson wrote:
> This is critically important as various parts of the compiler will take a
> degenerate PHI node and propagate the RHS of the PHI into the uses of the
> LHS of the PHI -- without doing any conversions.

I think this is OK, because tree-outof-ssa does send code in basic
blocks through expand_expr, which will emit conversions if necessary.
it is only the conversion of PHI nodes to RTL that is the problem, as
it doesn't use expand_expr, and hence doesn't get the
SUBREG_PROMOTED_P conversions.

Jim

Re: [PATCH, ARM] stop changing signedness in PROMOTE_MODE

[Jim Wilson]

On Tue, Jul 7, 2015 at 2:35 PM, Richard Biener
 wrote:
> On July 7, 2015 6:29:21 PM GMT+02:00, Jim Wilson  
> wrote:
>>signed sub-word locals.  Thus to detect the need for a conversion, you
>>have to have the decls, and we don't have them here.  There is also
>
> It probably is.  The decks for the parameter based SSA names are available, 
> for the PHI destination there might be no decl.

I tried looking again, and found the decls.  I'm able to get correct
code for my testcase with the attached patch to force the conversion.
It is rather inelegant, but I think I can cache the values I need to
make this simpler and cleaner.  I still don't have decls from
insert_part_to_rtx_on_edge and insert_rtx_to_part_on_edge, but it
looks like those are for breaking cycles, and hence might not need
conversions.

Jim
Index: tree-outof-ssa.c
===
--- tree-outof-ssa.c	(revision 225477)
+++ tree-outof-ssa.c	(working copy)
@@ -230,11 +230,32 @@ set_location_for_edge (edge e)
SRC/DEST might be BLKmode memory locations SIZEEXP is a tree from
which we deduce the size to copy in that case.  */
 
-static inline rtx_insn *
-emit_partition_copy (rtx dest, rtx src, int unsignedsrcp, tree sizeexp)
+rtx_insn *
+emit_partition_copy (rtx dest, rtx src, int unsignedsrcp, tree sizeexp,
+		 tree var2 ATTRIBUTE_UNUSED)
 {
   start_sequence ();
 
+  /* If var2 is set, then sizeexp is the src decl and var2 is the dest decl.  */
+  if (var2)
+{
+  tree src_var = (TREE_CODE (sizeexp) == SSA_NAME
+		  ? SSA_NAME_VAR (sizeexp) : sizeexp);
+  tree dest_var = (TREE_CODE (var2) == SSA_NAME
+		   ? SSA_NAME_VAR (var2) : var2);
+  int src_unsignedp = TYPE_UNSIGNED (TREE_TYPE (src_var));
+  int dest_unsignedp = TYPE_UNSIGNED (TREE_TYPE (dest_var));
+  machine_mode src_mode = promote_decl_mode (src_var, &src_unsignedp);
+  machine_mode dest_mode = promote_decl_mode (dest_var, &dest_unsignedp);
+  if (src_unsignedp != dest_unsignedp
+	  && src_mode != DECL_MODE (src_var)
+	  && dest_mode != DECL_MODE (dest_var))
+	{
+	  src = gen_lowpart_common (DECL_MODE (src_var), src);
+	  unsignedsrcp = dest_unsignedp;
+	}
+}
+
   if (GET_MODE (src) != VOIDmode && GET_MODE (src) != GET_MODE (dest))
 src = convert_to_mode (GET_MODE (dest), src, unsignedsrcp);
   if (GET_MODE (src) == BLKmode)
@@ -256,7 +277,7 @@ emit_partition_copy (rtx dest, rtx src,
 static void
 insert_partition_copy_on_edge (edge e, int dest, int src, source_location locus)
 {
-  tree var;
+  tree var, var2;
   if (dump_file && (dump_flags & TDF_DETAILS))
 {
   fprintf (dump_file,
@@ -276,10 +297,11 @@ insert_partition_copy_on_edge (edge e, i
 set_curr_insn_location (locus);
 
   var = partition_to_var (SA.map, src);
+  var2 = partition_to_var (SA.map, dest);
   rtx_insn *seq = emit_partition_copy (copy_rtx (SA.partition_to_pseudo[dest]),
    copy_rtx (SA.partition_to_pseudo[src]),
    TYPE_UNSIGNED (TREE_TYPE (var)),
-   var);
+   var, var2);
 
   insert_insn_on_edge (seq, e);
 }
@@ -373,7 +395,8 @@ insert_rtx_to_part_on_edge (edge e, int
  involved), so it doesn't matter.  */
   rtx_insn *seq = emit_partition_copy (copy_rtx (SA.partition_to_pseudo[dest]),
    src, unsignedsrcp,
-   partition_to_var (SA.map, dest));
+   partition_to_var (SA.map, dest), 0);
+
 
   insert_insn_on_edge (seq, e);
 }
@@ -406,7 +429,7 @@ insert_part_to_rtx_on_edge (edge e, rtx
   rtx_insn *seq = emit_partition_copy (dest,
    copy_rtx (SA.partition_to_pseudo[src]),
    TYPE_UNSIGNED (TREE_TYPE (var)),
-   var);
+   var, 0);
 
   insert_insn_on_edge (seq, e);
 }

Re: [PATCH, ARM] stop changing signedness in PROMOTE_MODE

[Jim Wilson]

On Tue, Jul 14, 2015 at 9:13 AM, Richard Earnshaw
 wrote:
> We went through this a couple of weeks back.  The backend documentation
> for PROMOTE_MODE says:

I disagree that this is a fully resolved issue.  I see clear problems
with how the ARM port uses PROMOTE_MODE.

> " For most machines, the macro definition does not change @var{unsignedp}.
> However, some machines, have instructions that preferentially handle
> either signed or unsigned quantities of certain modes.  For example, on
> the DEC Alpha, 32-bit loads from memory and 32-bit add instructions
> sign-extend the result to 64 bits.  On such machines, set
> @var{unsignedp} according to which kind of extension is more efficient."

The Alpha case is different than the ARM case.  The Alpha only changes
sign for 32-bit values in 64-bit registers.  The alpha happens to have
a nice set of instructions that operates on 32-bit values, that
accepts these wrong-signed values and handle them correctly.  Thus on
the alpha, it appears that there are no extra zero/sign extends
required, and everything is the same speed or faster with the wrong
sign.  The MIPS port works the same way.  This is not true on the arm
though.  The arm port is changing sign on 8 and 16 bit value, but does
not have instructions that directly operate on 8 and 16 bit values.
This requires the compiler to emit extra zero/sign extend instructions
that affect the performance of the code.  Other than the thumb1 8-bit
load, and the pre-armv6 use of andi for 8-bit zero-extend, I haven't
seen anything that is faster in the wrong sign, and there are a number
of operations that are actually slower because of the extra zero/sign
extends required by the arm PROMOTE_MODE definition.  I've given some
examples.

I have since noticed that the relatively new pass to optimize away
unnecessary zero/sign extensions is actually fixing some of the damage
caused by the ARM PROMOTE_MODE definition.  But there are still some
cases that won't get fixed, as per my earlier examples.  It is better
to emit the fast code at the beginning than to rely on an optimization
pass to convert the slow code into fast code.  So I think the ARM
PROMOTE_MODE definition still needs to be fixed.

> So clearly it anticipates that all permitted extensions should work, and
> in particular it makes no mention of this having to match some
> abi-mandated promotions.  That makes this a MI bug not a target one.

If you look at older gcc releases, like gcc-2.95.3, you will see that
there is only PROMOTE_MODE and a boolean PROMOTE_FUNCTION_ARGS which
says whether PROMOTE_MODE is applied to function arguments or not.
You can't have different zero/sign extensions for parameters and
locals in gcc-2.95.3.  The fact that you can have it today is more an
accident than a deliberate choice.  When PROMOTE_FUNCTION_ARGS was
hookized, it was made into a separate function, and for the ARM port,
the code for PROMOTE_MODE was not correctly copied into the new hook,
resulting in the accidental difference for parameter and local
zero/sign promotion for ARM.  The PROMOTE_MODE docs were written back
when different sign/zero extensions for parms/locals weren't possible,
and hence did not consider the case.

Now that we do have the problem, we can't fix it without an ARM port
ABI change, which is undesirable, so we may have to fix it with a MI
change.

There were two MI changes suggested, one was fixing the out-of-ssa
pass to handle SUBREG_PROMOTED_P promotions.  The other was to
disallow creating PHI nodes between parms and locals.  I haven't had a
chance to try implementing the second one yet; I hope to work on that
today.

Jim

Re: [PATCH, ARM] stop changing signedness in PROMOTE_MODE

[Jim Wilson]

On Wed, Jul 15, 2015 at 6:04 AM, Michael Matz  wrote:
> Hi,
>
> On Tue, 14 Jul 2015, Jim Wilson wrote:
>
>> Now that we do have the problem, we can't fix it without an ARM port ABI
>> change, which is undesirable, so we may have to fix it with a MI change.
>
> What's the ABI implication of fixing the inconsistency?

Currently signed chars and signed shorts are passed sign-extended.  If
we make TARGET_PROMOTE_FUNCTION_MODE work the same as PROMOTE_MODE,
then they will be passed zero-extended.

Given the testcase:

int sub (int) __attribute__ ((noinline));
int sub2 (signed char) __attribute__ ((noinline));
int sub (int i) { return sub2 (i); }
int sub2 (signed char c) { return c & 0xff; }

Currently sub will do a char sign-extend to convert the int to signed
char, and sub2 will do a char zero-extend for the and.  With the
change, sub will do a char zero-extend to convert the int to unsigned
char, and sub2 will do nothing.  If you compile sub without the change
and sub2 with the change, then you lose the and operation and get a
sign-extended char at the end.

>> There were two MI changes suggested, one was fixing the out-of-ssa pass
>> to handle SUBREG_PROMOTED_P promotions.  The other was to disallow
>> creating PHI nodes between parms and locals.  I haven't had a chance to
>> try implementing the second one yet; I hope to work on that today.
>
> Don't bother with the latter, it doesn't have a chance of being accepted.

I tried looking at it anyways, as I need to learn more about this
stuff.  It didn't seem feasible without changing a lot of optimization
passes which doesn't seem reasonable.

> If the terrible hack in outof-ssa really will be necessary (and I really
> really hope it won't) then I think I prefer the approach you partly tried
> in comment #12 of PR 65932 already.  Let partition_to_pseudo[] refer to
> the promoted subreg and deal with that situation in emit_partition_copy;
> I'd then hope that the unsignedsrcp parameter could go away (unfortunately
> the sizeexp will have to stay).

Yes, I think that is a cleaner way to do it, but I had trouble getting
that to work as I don't know enough about the code yet.  Doing it
directly in emit_partition_copy was easier, just to prove it could
work.  I can go back and try to make this work again.

Jim

[PATCH] [C] fix for ICE with -g

[Jim Wilson]

Compiling a simple testcase that defines an incomplete struct/union
and then a variant of that type and then completes the struct/union
gives an ICE in verify_type.

palantir:2257$ cat tmp.c
struct S a;
const struct S b;
struct S
{
};
palantir:2258$ ./xgcc -B./ -O -g tmp.c
tmp.c:5:1: error: type variant has different TYPE_VFIELD
 };
 ^
...
tmp.c:5:1: internal compiler error: verify_type failed
...

The problem is in the C front end.  It uses TYPE_VFIELD to keep track
of incomplete types via C_TYPE_INCOMPLETE_VARS.  This is only valid on
the type main variant, and is cleared when the type main variant is
completed.  When we create a variant type, this field is being copied
to the variant along with all other type fields.  Since we never look
at this field on variant types, it never gets cleared, and we end up
with dangling pointers that trigger the ICE in verify_type.  So this
should be fixed by clearing the field when creating a variant type.

Attached is a patch that does this clearing, and adds a testcase.
This was tested with an x86_64 linux bootstrap and make check, and
also a gdb make check.

With this patch, some of the hacks in type_verify to work around the C
front-end problem may no longer be necessary.  I haven't looked at
that.

Jim
Index: gcc/c/ChangeLog
===
--- gcc/c/ChangeLog	(revision 229395)
+++ gcc/c/ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2015-10-26  Jim Wilson  
+
+	PR debug/66068
+	* c-typeck.c (c_build_qualified_type): Clear C_TYPE_INCOMPLETE_VARS
+	after calling build_qualified_type.
+
 2015-10-22  Richard Biener  
 
 	* c-typeck.c (c_finish_omp_clauses): Properly convert operands
Index: gcc/c/c-typeck.c
===
--- gcc/c/c-typeck.c	(revision 229395)
+++ gcc/c/c-typeck.c	(working copy)
@@ -13090,6 +13090,8 @@ c_finish_transaction (location_t loc, tree block,
 tree
 c_build_qualified_type (tree type, int type_quals)
 {
+  tree var_type;
+
   if (type == error_mark_node)
 return type;
 
@@ -13146,7 +13148,13 @@ c_build_qualified_type (tree type, int type_quals)
   type_quals &= ~TYPE_QUAL_RESTRICT;
 }
 
-  return build_qualified_type (type, type_quals);
+  var_type = build_qualified_type (type, type_quals);
+  /* A variant type does not inherit the list of incomplete vars from the
+ type main variant.  */
+  if (TREE_CODE (var_type) == RECORD_TYPE
+  || TREE_CODE (var_type) == UNION_TYPE)
+C_TYPE_INCOMPLETE_VARS (var_type) = 0;
+  return var_type;
 }
 
 /* Build a VA_ARG_EXPR for the C parser.  */
Index: gcc/testsuite/ChangeLog
===
--- gcc/testsuite/ChangeLog	(revision 229395)
+++ gcc/testsuite/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2015-10-26  Jim Wilson  
+
+	PR debug/66068
+	* gcc.dg/debug/pr66068.c: New test.
+
 2015-10-26  Louis Krupp  
 
 	PR fortran/66056
Index: gcc/testsuite/gcc.dg/debug/pr66068.c
===
--- gcc/testsuite/gcc.dg/debug/pr66068.c	(revision 0)
+++ gcc/testsuite/gcc.dg/debug/pr66068.c	(working copy)
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+
+struct S a;
+const struct S b;
+struct S
+{
+};
+
+union U c;
+const union U d;
+union U
+{
+};