* [PATCH] Rewrite pic.md to improve medany and pic code size.
@ 2018-08-29 2:21 Jim Wilson
2018-08-29 16:22 ` Palmer Dabbelt
0 siblings, 1 reply; 4+ messages in thread
From: Jim Wilson @ 2018-08-29 2:21 UTC (permalink / raw)
To: gcc-patches; +Cc: Jim Wilson
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(-)
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")])
--
2.17.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Rewrite pic.md to improve medany and pic code size.
2018-08-29 2:21 [PATCH] Rewrite pic.md to improve medany and pic code size Jim Wilson
@ 2018-08-29 16:22 ` Palmer Dabbelt
2018-08-29 16:52 ` Jim Wilson
0 siblings, 1 reply; 4+ messages in thread
From: Palmer Dabbelt @ 2018-08-29 16:22 UTC (permalink / raw)
To: Jim Wilson; +Cc: gcc-patches
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")])
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Rewrite pic.md to improve medany and pic code size.
2018-08-29 16:22 ` Palmer Dabbelt
@ 2018-08-29 16:52 ` Jim Wilson
2018-08-29 23:46 ` Palmer Dabbelt
0 siblings, 1 reply; 4+ messages in thread
From: Jim Wilson @ 2018-08-29 16:52 UTC (permalink / raw)
To: Palmer Dabbelt; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Rewrite pic.md to improve medany and pic code size.
2018-08-29 16:52 ` Jim Wilson
@ 2018-08-29 23:46 ` Palmer Dabbelt
0 siblings, 0 replies; 4+ messages in thread
From: Palmer Dabbelt @ 2018-08-29 23:46 UTC (permalink / raw)
To: Jim Wilson; +Cc: gcc-patches
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!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-08-29 23:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-29 2:21 [PATCH] Rewrite pic.md to improve medany and pic code size Jim Wilson
2018-08-29 16:22 ` Palmer Dabbelt
2018-08-29 16:52 ` Jim Wilson
2018-08-29 23:46 ` Palmer Dabbelt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).