Rewrite pic.md to improve medany and pic code size.

Message ID 20180829022123.25863-1-jimw@sifive.com
State New
Headers show
Series
  • Rewrite pic.md to improve medany and pic code size.
Related show

Commit Message

Jim Wilson Aug. 29, 2018, 2:21 a.m.
The pic.md file has patterns used only for the medany code model and for pic
code.  They match an unsplit 2-instruction address load pattern followed by
a load or store instruction, and emit an assembler macro that expands to two
instructions.  This replaces 3 instructions with 2.  Unfortunately, there
are a lot of broken patterns in the file.  It has things like
   (sign_extend:ANYI (mem:ANYI ...)
which can't work, as the sign_extend needs to be a larger mode than the mem.
There are also a lot of missing patterns.  It has patterns for sign and zero
extending loads, but not regular loads for instance.  Fixing these problems
required a major rewrite of the file.  While doing this, I noticed a case
that results in worse code, and had to change address costs to fix that.
I also ended up adding a number of new mode attributes and mode iterators
to support the new pic.md file.

This was tested with cross rv{32,64}-{elf,linux} builds and checks using the
medany code model.  There were no regressions.  I'm seeing about a 0.3% to 0.4%
code size reduction for newlib/glibc libc.a/libc.so.  This was also tested
with a cross kernel build and boot on qemu to verify that I didn't break kernel
builds.

Committed.

Jim

	gcc/
	* config/riscv/pic.md: Rewrite.
	* config/riscv/riscv.c (riscv_address_insns): Return cost of 3 for
	invalid address.
	* config/riscv/riscv.md (ZERO_EXTEND_LOAD): Delete.
	(SOFTF, default_load, softload, softstore): New.
---
 gcc/config/riscv/pic.md   | 113 ++++++++++++++++++++++++--------------
 gcc/config/riscv/riscv.c  |   8 ++-
 gcc/config/riscv/riscv.md |  16 +++++-
 3 files changed, 91 insertions(+), 46 deletions(-)

-- 
2.17.1

Comments

Palmer Dabbelt Aug. 29, 2018, 4:22 p.m. | #1
On Tue, 28 Aug 2018 19:21:23 PDT (-0700), Jim Wilson wrote:
> The pic.md file has patterns used only for the medany code model and for pic

> code.  They match an unsplit 2-instruction address load pattern followed by

> a load or store instruction, and emit an assembler macro that expands to two

> instructions.  This replaces 3 instructions with 2.  Unfortunately, there

> are a lot of broken patterns in the file.  It has things like

>    (sign_extend:ANYI (mem:ANYI ...)

> which can't work, as the sign_extend needs to be a larger mode than the mem.

> There are also a lot of missing patterns.  It has patterns for sign and zero

> extending loads, but not regular loads for instance.  Fixing these problems

> required a major rewrite of the file.  While doing this, I noticed a case

> that results in worse code, and had to change address costs to fix that.

> I also ended up adding a number of new mode attributes and mode iterators

> to support the new pic.md file.


Thanks Jim -- I'm afraid at least part of this was my mess, as I had to go add 
in the ZERO_EXTEND_LOAD hackery to work around some bug in this file that I 
couldn't figure out how to fix in a better way.

> This was tested with cross rv{32,64}-{elf,linux} builds and checks using the

> medany code model.  There were no regressions.  I'm seeing about a 0.3% to 0.4%

> code size reduction for newlib/glibc libc.a/libc.so.  This was also tested

> with a cross kernel build and boot on qemu to verify that I didn't break kernel

> builds.


IIRC the issue I found was in booting a Linux kernel that was built with 
"-mcmodel=medany".  If I'm reading our current kernel port correctly that's the 
default for 64-bit systems, but I'm not sure when we flipped over so I think 
it's worth checking -- I know at the time I fixed the bug it wasn't the default 
:).  You should be able to do a "make V=1" inside Linux to just see the 
compiler command lines.

> Committed.


Thanks for the fix!

>

> Jim

>

> 	gcc/

> 	* config/riscv/pic.md: Rewrite.

> 	* config/riscv/riscv.c (riscv_address_insns): Return cost of 3 for

> 	invalid address.

> 	* config/riscv/riscv.md (ZERO_EXTEND_LOAD): Delete.

> 	(SOFTF, default_load, softload, softstore): New.

> ---

>  gcc/config/riscv/pic.md   | 113 ++++++++++++++++++++++++--------------

>  gcc/config/riscv/riscv.c  |   8 ++-

>  gcc/config/riscv/riscv.md |  16 +++++-

>  3 files changed, 91 insertions(+), 46 deletions(-)

>

> diff --git a/gcc/config/riscv/pic.md b/gcc/config/riscv/pic.md

> index a4a9732656c..942502058e0 100644

> --- a/gcc/config/riscv/pic.md

> +++ b/gcc/config/riscv/pic.md

> @@ -22,71 +22,100 @@

>  ;; Simplify PIC loads to static variables.

>  ;; These should go away once we figure out how to emit auipc discretely.

>

> -(define_insn "*local_pic_load_s<mode>"

> +(define_insn "*local_pic_load<mode>"

>    [(set (match_operand:ANYI 0 "register_operand" "=r")

> -	(sign_extend:ANYI (mem:ANYI (match_operand 1 "absolute_symbolic_operand" ""))))]

> +	(mem:ANYI (match_operand 1 "absolute_symbolic_operand" "")))]

> +  "USE_LOAD_ADDRESS_MACRO (operands[1])"

> +  "<default_load>\t%0,%1"

> +  [(set (attr "length") (const_int 8))])

> +

> +(define_insn "*local_pic_load_s<mode>"

> +  [(set (match_operand:SUPERQI 0 "register_operand" "=r")

> +	(sign_extend:SUPERQI (mem:SUBX (match_operand 1 "absolute_symbolic_operand" ""))))]

>    "USE_LOAD_ADDRESS_MACRO (operands[1])"

> -  "<load>\t%0,%1"

> +  "<SUBX:load>\t%0,%1"

>    [(set (attr "length") (const_int 8))])

>

>  (define_insn "*local_pic_load_u<mode>"

> -  [(set (match_operand:ZERO_EXTEND_LOAD 0 "register_operand" "=r")

> -	(zero_extend:ZERO_EXTEND_LOAD (mem:ZERO_EXTEND_LOAD (match_operand 1 "absolute_symbolic_operand" ""))))]

> +  [(set (match_operand:SUPERQI 0 "register_operand" "=r")

> +	(zero_extend:SUPERQI (mem:SUBX (match_operand 1 "absolute_symbolic_operand" ""))))]

>    "USE_LOAD_ADDRESS_MACRO (operands[1])"

> -  "<load>u\t%0,%1"

> +  "<SUBX:load>u\t%0,%1"

>    [(set (attr "length") (const_int 8))])

>

> -(define_insn "*local_pic_load<mode>"

> -  [(set (match_operand:ANYF 0 "register_operand" "=f")

> +;; We can support ANYF loads into X register if there is no double support

> +;; or if the target is 64-bit.

> +

> +(define_insn "*local_pic_load<ANYF:mode>"

> +  [(set (match_operand:ANYF 0 "register_operand" "=f,*r")

>  	(mem:ANYF (match_operand 1 "absolute_symbolic_operand" "")))

> -   (clobber (match_scratch:DI 2 "=r"))]

> -  "TARGET_HARD_FLOAT && TARGET_64BIT && USE_LOAD_ADDRESS_MACRO (operands[1])"

> -  "<load>\t%0,%1,%2"

> +   (clobber (match_scratch:P 2 "=r,X"))]

> +  "TARGET_HARD_FLOAT && USE_LOAD_ADDRESS_MACRO (operands[1])

> +   && (!TARGET_DOUBLE_FLOAT || TARGET_64BIT)"

> +  "@

> +   <ANYF:load>\t%0,%1,%2

> +   <softload>\t%0,%1"

>    [(set (attr "length") (const_int 8))])

>

> -(define_insn "*local_pic_load<mode>"

> +;; ??? For a 32-bit target with double float, a DF load into a X reg isn't

> +;; supported.  ld is not valid in that case.  Punt for now.  Maybe add a split

> +;; for this later.

> +

> +(define_insn "*local_pic_load_32d<ANYF:mode>"

>    [(set (match_operand:ANYF 0 "register_operand" "=f")

>  	(mem:ANYF (match_operand 1 "absolute_symbolic_operand" "")))

> -   (clobber (match_scratch:SI 2 "=r"))]

> -  "TARGET_HARD_FLOAT && !TARGET_64BIT && USE_LOAD_ADDRESS_MACRO (operands[1])"

> -  "<load>\t%0,%1,%2"

> +   (clobber (match_scratch:P 2 "=r"))]

> +  "TARGET_HARD_FLOAT && USE_LOAD_ADDRESS_MACRO (operands[1])

> +   && (TARGET_DOUBLE_FLOAT && !TARGET_64BIT)"

> +  "<ANYF:load>\t%0,%1,%2"

>    [(set (attr "length") (const_int 8))])

>

> -(define_insn "*local_pic_loadu<mode>"

> -  [(set (match_operand:SUPERQI 0 "register_operand" "=r")

> -	(zero_extend:SUPERQI (mem:SUBX (match_operand 1 "absolute_symbolic_operand" ""))))]

> -  "USE_LOAD_ADDRESS_MACRO (operands[1])"

> -  "<load>u\t%0,%1"

> +(define_insn "*local_pic_load_sf<mode>"

> +  [(set (match_operand:SOFTF 0 "register_operand" "=r")

> +	(mem:SOFTF (match_operand 1 "absolute_symbolic_operand" "")))]

> +  "!TARGET_HARD_FLOAT && USE_LOAD_ADDRESS_MACRO (operands[1])"

> +  "<softload>\t%0,%1"

>    [(set (attr "length") (const_int 8))])

>

> -(define_insn "*local_pic_storedi<mode>"

> -  [(set (mem:ANYI (match_operand 0 "absolute_symbolic_operand" ""))

> -	(match_operand:ANYI 1 "reg_or_0_operand" "rJ"))

> -   (clobber (match_scratch:DI 2 "=&r"))]

> -  "TARGET_64BIT && USE_LOAD_ADDRESS_MACRO (operands[0])"

> -  "<store>\t%z1,%0,%2"

> -  [(set (attr "length") (const_int 8))])

> +;; Simplify PIC stores to static variables.

> +;; These should go away once we figure out how to emit auipc discretely.

>

> -(define_insn "*local_pic_storesi<mode>"

> +(define_insn "*local_pic_store<ANYI:mode>"

>    [(set (mem:ANYI (match_operand 0 "absolute_symbolic_operand" ""))

>  	(match_operand:ANYI 1 "reg_or_0_operand" "rJ"))

> -   (clobber (match_scratch:SI 2 "=&r"))]

> -  "!TARGET_64BIT && USE_LOAD_ADDRESS_MACRO (operands[0])"

> -  "<store>\t%z1,%0,%2"

> +   (clobber (match_scratch:P 2 "=&r"))]

> +  "USE_LOAD_ADDRESS_MACRO (operands[0])"

> +  "<ANYI:store>\t%z1,%0,%2"

>    [(set (attr "length") (const_int 8))])

>

> -(define_insn "*local_pic_storedi<mode>"

> +(define_insn "*local_pic_store<ANYF:mode>"

>    [(set (mem:ANYF (match_operand 0 "absolute_symbolic_operand" ""))

> -	(match_operand:ANYF 1 "register_operand" "f"))

> -   (clobber (match_scratch:DI 2 "=r"))]

> -  "TARGET_HARD_FLOAT && TARGET_64BIT && USE_LOAD_ADDRESS_MACRO (operands[0])"

> -  "<store>\t%1,%0,%2"

> +	(match_operand:ANYF 1 "register_operand" "f,*r"))

> +   (clobber (match_scratch:P 2 "=r,&r"))]

> +  "TARGET_HARD_FLOAT && USE_LOAD_ADDRESS_MACRO (operands[0])

> +   && (!TARGET_DOUBLE_FLOAT || TARGET_64BIT)"

> +  "@

> +   <ANYF:store>\t%1,%0,%2

> +   <softstore>\t%1,%0,%2"

>    [(set (attr "length") (const_int 8))])

>

> -(define_insn "*local_pic_storesi<mode>"

> -  [(set (mem:ANYF (match_operand 0 "absolute_symbolic_operand" ""))

> -	(match_operand:ANYF 1 "register_operand" "f"))

> -   (clobber (match_scratch:SI 2 "=r"))]

> -  "TARGET_HARD_FLOAT && !TARGET_64BIT && USE_LOAD_ADDRESS_MACRO (operands[0])"

> -  "<store>\t%1,%0,%2"

> +;; ??? For a 32-bit target with double float, a DF store from a X reg isn't

> +;; supported.  sd is not valid in that case.  Punt for now.  Maybe add a split

> +;; for this later.

> +

> +(define_insn "*local_pic_store_32d<ANYF:mode>"

> +  [(set (match_operand:ANYF 0 "register_operand" "=f")

> +	(mem:ANYF (match_operand 1 "absolute_symbolic_operand" "")))

> +   (clobber (match_scratch:P 2 "=r"))]

> +  "TARGET_HARD_FLOAT && USE_LOAD_ADDRESS_MACRO (operands[1])

> +   && (TARGET_DOUBLE_FLOAT && !TARGET_64BIT)"

> +  "<ANYF:store>\t%1,%0,%2"

> +  [(set (attr "length") (const_int 8))])

> +

> +(define_insn "*local_pic_store_sf<SOFTF:mode>"

> +  [(set (mem:SOFTF (match_operand 0 "absolute_symbolic_operand" ""))

> +	(match_operand:SOFTF 1 "register_operand" "r"))

> +   (clobber (match_scratch:P 2 "=&r"))]

> +  "!TARGET_HARD_FLOAT && USE_LOAD_ADDRESS_MACRO (operands[0])"

> +  "<softstore>\t%1,%0,%2"

>    [(set (attr "length") (const_int 8))])

> diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c

> index 69e70feaf33..9d6d981a42a 100644

> --- a/gcc/config/riscv/riscv.c

> +++ b/gcc/config/riscv/riscv.c

> @@ -802,7 +802,13 @@ riscv_address_insns (rtx x, machine_mode mode, bool might_split_p)

>    int n = 1;

>

>    if (!riscv_classify_address (&addr, x, mode, false))

> -    return 0;

> +    {

> +      /* This could be a pattern from the pic.md file.  In which case we want

> +	 this address to always have a cost of 3 to make it as expensive as the

> +	 most expensive symbol.  This prevents constant propagation from

> +	 preferring symbols over register plus offset.  */

> +      return 3;

> +    }

>

>    /* BLKmode is used for single unaligned loads and stores and should

>       not count as a multiword mode. */

> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md

> index 613af9d79e4..95fbb282c7c 100644

> --- a/gcc/config/riscv/riscv.md

> +++ b/gcc/config/riscv/riscv.md

> @@ -269,9 +269,6 @@

>  ;; Iterator for QImode extension patterns.

>  (define_mode_iterator SUPERQI [HI SI (DI "TARGET_64BIT")])

>

> -;; Iterator for extending loads.

> -(define_mode_iterator ZERO_EXTEND_LOAD [QI HI (SI "TARGET_64BIT")])

> -

>  ;; Iterator for hardware integer modes narrower than XLEN.

>  (define_mode_iterator SUBX [QI HI (SI "TARGET_64BIT")])

>

> @@ -282,6 +279,9 @@

>  (define_mode_iterator ANYF [(SF "TARGET_HARD_FLOAT")

>  			    (DF "TARGET_DOUBLE_FLOAT")])

>

> +;; Iterator for floating-point modes that can be loaded into X registers.

> +(define_mode_iterator SOFTF [SF (DF "TARGET_64BIT")])

> +

>  ;; This attribute gives the length suffix for a sign- or zero-extension

>  ;; instruction.

>  (define_mode_attr size [(QI "b") (HI "h")])

> @@ -289,9 +289,19 @@

>  ;; Mode attributes for loads.

>  (define_mode_attr load [(QI "lb") (HI "lh") (SI "lw") (DI "ld") (SF "flw") (DF "fld")])

>

> +;; Instruction names for integer loads that aren't explicitly sign or zero

> +;; extended.  See riscv_output_move and LOAD_EXTEND_OP.

> +(define_mode_attr default_load [(QI "lbu") (HI "lhu") (SI "lw") (DI "ld")])

> +

> +;; Mode attribute for FP loads into integer registers.

> +(define_mode_attr softload [(SF "lw") (DF "ld")])

> +

>  ;; Instruction names for stores.

>  (define_mode_attr store [(QI "sb") (HI "sh") (SI "sw") (DI "sd") (SF "fsw") (DF "fsd")])

>

> +;; Instruction names for FP stores from integer registers.

> +(define_mode_attr softstore [(SF "sw") (DF "sd")])

> +

>  ;; This attribute gives the best constraint to use for registers of

>  ;; a given mode.

>  (define_mode_attr reg [(SI "d") (DI "d") (CC "d")])
Jim Wilson Aug. 29, 2018, 4:52 p.m. | #2
On Wed, Aug 29, 2018 at 9:22 AM, Palmer Dabbelt <palmer@sifive.com> wrote:
> Thanks Jim -- I'm afraid at least part of this was my mess, as I had to go

> add in the ZERO_EXTEND_LOAD hackery to work around some bug in this file

> that I couldn't figure out how to fix in a better way.


ZERO_EXTEND_LOAD is exactly the same as SUBX, so is redundant.  That
is the main reason why I dropped it.

> IIRC the issue I found was in booting a Linux kernel that was built with

> "-mcmodel=medany".  If I'm reading our current kernel port correctly that's

> the default for 64-bit systems, but I'm not sure when we flipped over so I

> think it's worth checking -- I know at the time I fixed the bug it wasn't

> the default :).  You should be able to do a "make V=1" inside Linux to just

> see the compiler command lines.


By default, 32-bit kernels are built medlow, and 64-bit kernels are
built medany.  That is in the riscv Kconfig file.  Before committing
the patch, I did apply it to freedom-u-sdk, build a 64-bit kernel, and
boot both on qemu and spike, so I think we are OK here.

I did find a few optimization problems via the gcc testsuite while
trying to improve the file.  The most interesting one, and maybe the
one you hit earlier, is with non-extended loads, which because of
LOAD_EXTEND_OP we actually have to use zero-extending loads for
char/short loads, but sign-extending loads for words, and I had to add
a new mode attribute for that.  That one took a little time to debug
because it didn't fail with trivial testcases, and the miscompilation
was non-obvious.

Jim
Palmer Dabbelt Aug. 29, 2018, 11:46 p.m. | #3
On Wed, 29 Aug 2018 09:52:00 PDT (-0700), Jim Wilson wrote:
> On Wed, Aug 29, 2018 at 9:22 AM, Palmer Dabbelt <palmer@sifive.com> wrote:

>> Thanks Jim -- I'm afraid at least part of this was my mess, as I had to go

>> add in the ZERO_EXTEND_LOAD hackery to work around some bug in this file

>> that I couldn't figure out how to fix in a better way.

>

> ZERO_EXTEND_LOAD is exactly the same as SUBX, so is redundant.  That

> is the main reason why I dropped it.


Ah, OK, this makes a bit more sense then.

>> IIRC the issue I found was in booting a Linux kernel that was built with

>> "-mcmodel=medany".  If I'm reading our current kernel port correctly that's

>> the default for 64-bit systems, but I'm not sure when we flipped over so I

>> think it's worth checking -- I know at the time I fixed the bug it wasn't

>> the default :).  You should be able to do a "make V=1" inside Linux to just

>> see the compiler command lines.

>

> By default, 32-bit kernels are built medlow, and 64-bit kernels are

> built medany.  That is in the riscv Kconfig file.  Before committing

> the patch, I did apply it to freedom-u-sdk, build a 64-bit kernel, and

> boot both on qemu and spike, so I think we are OK here.


Yep, as long as that's the case for whatever kernel you built then we're OK.

> I did find a few optimization problems via the gcc testsuite while

> trying to improve the file.  The most interesting one, and maybe the

> one you hit earlier, is with non-extended loads, which because of

> LOAD_EXTEND_OP we actually have to use zero-extending loads for

> char/short loads, but sign-extending loads for words, and I had to add

> a new mode attribute for that.  That one took a little time to debug

> because it didn't fail with trivial testcases, and the miscompilation

> was non-obvious.


That sounds very much like what I ran in to.

Thanks!

Patch

diff --git a/gcc/config/riscv/pic.md b/gcc/config/riscv/pic.md
index a4a9732656c..942502058e0 100644
--- a/gcc/config/riscv/pic.md
+++ b/gcc/config/riscv/pic.md
@@ -22,71 +22,100 @@ 
 ;; Simplify PIC loads to static variables.
 ;; These should go away once we figure out how to emit auipc discretely.
 
-(define_insn "*local_pic_load_s<mode>"
+(define_insn "*local_pic_load<mode>"
   [(set (match_operand:ANYI 0 "register_operand" "=r")
-	(sign_extend:ANYI (mem:ANYI (match_operand 1 "absolute_symbolic_operand" ""))))]
+	(mem:ANYI (match_operand 1 "absolute_symbolic_operand" "")))]
+  "USE_LOAD_ADDRESS_MACRO (operands[1])"
+  "<default_load>\t%0,%1"
+  [(set (attr "length") (const_int 8))])
+
+(define_insn "*local_pic_load_s<mode>"
+  [(set (match_operand:SUPERQI 0 "register_operand" "=r")
+	(sign_extend:SUPERQI (mem:SUBX (match_operand 1 "absolute_symbolic_operand" ""))))]
   "USE_LOAD_ADDRESS_MACRO (operands[1])"
-  "<load>\t%0,%1"
+  "<SUBX:load>\t%0,%1"
   [(set (attr "length") (const_int 8))])
 
 (define_insn "*local_pic_load_u<mode>"
-  [(set (match_operand:ZERO_EXTEND_LOAD 0 "register_operand" "=r")
-	(zero_extend:ZERO_EXTEND_LOAD (mem:ZERO_EXTEND_LOAD (match_operand 1 "absolute_symbolic_operand" ""))))]
+  [(set (match_operand:SUPERQI 0 "register_operand" "=r")
+	(zero_extend:SUPERQI (mem:SUBX (match_operand 1 "absolute_symbolic_operand" ""))))]
   "USE_LOAD_ADDRESS_MACRO (operands[1])"
-  "<load>u\t%0,%1"
+  "<SUBX:load>u\t%0,%1"
   [(set (attr "length") (const_int 8))])
 
-(define_insn "*local_pic_load<mode>"
-  [(set (match_operand:ANYF 0 "register_operand" "=f")
+;; We can support ANYF loads into X register if there is no double support
+;; or if the target is 64-bit.
+
+(define_insn "*local_pic_load<ANYF:mode>"
+  [(set (match_operand:ANYF 0 "register_operand" "=f,*r")
 	(mem:ANYF (match_operand 1 "absolute_symbolic_operand" "")))
-   (clobber (match_scratch:DI 2 "=r"))]
-  "TARGET_HARD_FLOAT && TARGET_64BIT && USE_LOAD_ADDRESS_MACRO (operands[1])"
-  "<load>\t%0,%1,%2"
+   (clobber (match_scratch:P 2 "=r,X"))]
+  "TARGET_HARD_FLOAT && USE_LOAD_ADDRESS_MACRO (operands[1])
+   && (!TARGET_DOUBLE_FLOAT || TARGET_64BIT)"
+  "@
+   <ANYF:load>\t%0,%1,%2
+   <softload>\t%0,%1"
   [(set (attr "length") (const_int 8))])
 
-(define_insn "*local_pic_load<mode>"
+;; ??? For a 32-bit target with double float, a DF load into a X reg isn't
+;; supported.  ld is not valid in that case.  Punt for now.  Maybe add a split
+;; for this later.
+
+(define_insn "*local_pic_load_32d<ANYF:mode>"
   [(set (match_operand:ANYF 0 "register_operand" "=f")
 	(mem:ANYF (match_operand 1 "absolute_symbolic_operand" "")))
-   (clobber (match_scratch:SI 2 "=r"))]
-  "TARGET_HARD_FLOAT && !TARGET_64BIT && USE_LOAD_ADDRESS_MACRO (operands[1])"
-  "<load>\t%0,%1,%2"
+   (clobber (match_scratch:P 2 "=r"))]
+  "TARGET_HARD_FLOAT && USE_LOAD_ADDRESS_MACRO (operands[1])
+   && (TARGET_DOUBLE_FLOAT && !TARGET_64BIT)"
+  "<ANYF:load>\t%0,%1,%2"
   [(set (attr "length") (const_int 8))])
 
-(define_insn "*local_pic_loadu<mode>"
-  [(set (match_operand:SUPERQI 0 "register_operand" "=r")
-	(zero_extend:SUPERQI (mem:SUBX (match_operand 1 "absolute_symbolic_operand" ""))))]
-  "USE_LOAD_ADDRESS_MACRO (operands[1])"
-  "<load>u\t%0,%1"
+(define_insn "*local_pic_load_sf<mode>"
+  [(set (match_operand:SOFTF 0 "register_operand" "=r")
+	(mem:SOFTF (match_operand 1 "absolute_symbolic_operand" "")))]
+  "!TARGET_HARD_FLOAT && USE_LOAD_ADDRESS_MACRO (operands[1])"
+  "<softload>\t%0,%1"
   [(set (attr "length") (const_int 8))])
 
-(define_insn "*local_pic_storedi<mode>"
-  [(set (mem:ANYI (match_operand 0 "absolute_symbolic_operand" ""))
-	(match_operand:ANYI 1 "reg_or_0_operand" "rJ"))
-   (clobber (match_scratch:DI 2 "=&r"))]
-  "TARGET_64BIT && USE_LOAD_ADDRESS_MACRO (operands[0])"
-  "<store>\t%z1,%0,%2"
-  [(set (attr "length") (const_int 8))])
+;; Simplify PIC stores to static variables.
+;; These should go away once we figure out how to emit auipc discretely.
 
-(define_insn "*local_pic_storesi<mode>"
+(define_insn "*local_pic_store<ANYI:mode>"
   [(set (mem:ANYI (match_operand 0 "absolute_symbolic_operand" ""))
 	(match_operand:ANYI 1 "reg_or_0_operand" "rJ"))
-   (clobber (match_scratch:SI 2 "=&r"))]
-  "!TARGET_64BIT && USE_LOAD_ADDRESS_MACRO (operands[0])"
-  "<store>\t%z1,%0,%2"
+   (clobber (match_scratch:P 2 "=&r"))]
+  "USE_LOAD_ADDRESS_MACRO (operands[0])"
+  "<ANYI:store>\t%z1,%0,%2"
   [(set (attr "length") (const_int 8))])
 
-(define_insn "*local_pic_storedi<mode>"
+(define_insn "*local_pic_store<ANYF:mode>"
   [(set (mem:ANYF (match_operand 0 "absolute_symbolic_operand" ""))
-	(match_operand:ANYF 1 "register_operand" "f"))
-   (clobber (match_scratch:DI 2 "=r"))]
-  "TARGET_HARD_FLOAT && TARGET_64BIT && USE_LOAD_ADDRESS_MACRO (operands[0])"
-  "<store>\t%1,%0,%2"
+	(match_operand:ANYF 1 "register_operand" "f,*r"))
+   (clobber (match_scratch:P 2 "=r,&r"))]
+  "TARGET_HARD_FLOAT && USE_LOAD_ADDRESS_MACRO (operands[0])
+   && (!TARGET_DOUBLE_FLOAT || TARGET_64BIT)"
+  "@
+   <ANYF:store>\t%1,%0,%2
+   <softstore>\t%1,%0,%2"
   [(set (attr "length") (const_int 8))])
 
-(define_insn "*local_pic_storesi<mode>"
-  [(set (mem:ANYF (match_operand 0 "absolute_symbolic_operand" ""))
-	(match_operand:ANYF 1 "register_operand" "f"))
-   (clobber (match_scratch:SI 2 "=r"))]
-  "TARGET_HARD_FLOAT && !TARGET_64BIT && USE_LOAD_ADDRESS_MACRO (operands[0])"
-  "<store>\t%1,%0,%2"
+;; ??? For a 32-bit target with double float, a DF store from a X reg isn't
+;; supported.  sd is not valid in that case.  Punt for now.  Maybe add a split
+;; for this later.
+
+(define_insn "*local_pic_store_32d<ANYF:mode>"
+  [(set (match_operand:ANYF 0 "register_operand" "=f")
+	(mem:ANYF (match_operand 1 "absolute_symbolic_operand" "")))
+   (clobber (match_scratch:P 2 "=r"))]
+  "TARGET_HARD_FLOAT && USE_LOAD_ADDRESS_MACRO (operands[1])
+   && (TARGET_DOUBLE_FLOAT && !TARGET_64BIT)"
+  "<ANYF:store>\t%1,%0,%2"
+  [(set (attr "length") (const_int 8))])
+
+(define_insn "*local_pic_store_sf<SOFTF:mode>"
+  [(set (mem:SOFTF (match_operand 0 "absolute_symbolic_operand" ""))
+	(match_operand:SOFTF 1 "register_operand" "r"))
+   (clobber (match_scratch:P 2 "=&r"))]
+  "!TARGET_HARD_FLOAT && USE_LOAD_ADDRESS_MACRO (operands[0])"
+  "<softstore>\t%1,%0,%2"
   [(set (attr "length") (const_int 8))])
diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index 69e70feaf33..9d6d981a42a 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -802,7 +802,13 @@  riscv_address_insns (rtx x, machine_mode mode, bool might_split_p)
   int n = 1;
 
   if (!riscv_classify_address (&addr, x, mode, false))
-    return 0;
+    {
+      /* This could be a pattern from the pic.md file.  In which case we want
+	 this address to always have a cost of 3 to make it as expensive as the
+	 most expensive symbol.  This prevents constant propagation from
+	 preferring symbols over register plus offset.  */
+      return 3;
+    }
 
   /* BLKmode is used for single unaligned loads and stores and should
      not count as a multiword mode. */
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 613af9d79e4..95fbb282c7c 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -269,9 +269,6 @@ 
 ;; Iterator for QImode extension patterns.
 (define_mode_iterator SUPERQI [HI SI (DI "TARGET_64BIT")])
 
-;; Iterator for extending loads.
-(define_mode_iterator ZERO_EXTEND_LOAD [QI HI (SI "TARGET_64BIT")])
-
 ;; Iterator for hardware integer modes narrower than XLEN.
 (define_mode_iterator SUBX [QI HI (SI "TARGET_64BIT")])
 
@@ -282,6 +279,9 @@ 
 (define_mode_iterator ANYF [(SF "TARGET_HARD_FLOAT")
 			    (DF "TARGET_DOUBLE_FLOAT")])
 
+;; Iterator for floating-point modes that can be loaded into X registers.
+(define_mode_iterator SOFTF [SF (DF "TARGET_64BIT")])
+
 ;; This attribute gives the length suffix for a sign- or zero-extension
 ;; instruction.
 (define_mode_attr size [(QI "b") (HI "h")])
@@ -289,9 +289,19 @@ 
 ;; Mode attributes for loads.
 (define_mode_attr load [(QI "lb") (HI "lh") (SI "lw") (DI "ld") (SF "flw") (DF "fld")])
 
+;; Instruction names for integer loads that aren't explicitly sign or zero
+;; extended.  See riscv_output_move and LOAD_EXTEND_OP.
+(define_mode_attr default_load [(QI "lbu") (HI "lhu") (SI "lw") (DI "ld")])
+
+;; Mode attribute for FP loads into integer registers.
+(define_mode_attr softload [(SF "lw") (DF "ld")])
+
 ;; Instruction names for stores.
 (define_mode_attr store [(QI "sb") (HI "sh") (SI "sw") (DI "sd") (SF "fsw") (DF "fsd")])
 
+;; Instruction names for FP stores from integer registers.
+(define_mode_attr softstore [(SF "sw") (DF "sd")])
+
 ;; This attribute gives the best constraint to use for registers of
 ;; a given mode.
 (define_mode_attr reg [(SI "d") (DI "d") (CC "d")])