Re: [PATCH], Improve PowerPC switch behavior on medium code model system

2018-08-20 Thread Michael Meissner
On Fri, Aug 10, 2018 at 11:04:50AM -0500, Segher Boessenkool wrote:
> On Tue, Jul 31, 2018 at 10:39:21AM -0400, Michael Meissner wrote:
> > This patch adds an insn to load a LABEL_REF into a GPR.  This is needed so 
> > the
> > FWPROP1 pass can convert the load the of the label address from the TOC to a
> > direct load to a GPR.
> 
> I don't see why you need a separate RTL insn for this.  It seems to me
> that some more generic pattern should accept label_refs.

I'm not aware of any.

> > While working on the patch, I discovered that the LWA instruction did not
> > support indexed loads.  This was due to it using the 'Y' constraint, which
> > accepts DS-form offsettable addresses, but not X-form indexed addresses.  I
> > added the Z constraint so that the indexed form is accepted.
> 
> This part is fine.  Please split it out to a separate patch.

I just added PR target/87033 for this, and I will submit the patch shortly.

> > * config/rs6000/rs6000.md (extendsi2): Allow reg+reg indexed
> > addressing.
> 
> This should say it is changing the constraints.
> 
> > (labelref): New insn to optimize loading a label address into
> > registers on a medium code system.
> 
> (*labelref) btw.

Yes.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797



Re: [PATCH], Improve PowerPC switch behavior on medium code model system

2018-08-10 Thread Segher Boessenkool
On Tue, Jul 31, 2018 at 10:39:21AM -0400, Michael Meissner wrote:
> This patch adds an insn to load a LABEL_REF into a GPR.  This is needed so the
> FWPROP1 pass can convert the load the of the label address from the TOC to a
> direct load to a GPR.

I don't see why you need a separate RTL insn for this.  It seems to me
that some more generic pattern should accept label_refs.

> While working on the patch, I discovered that the LWA instruction did not
> support indexed loads.  This was due to it using the 'Y' constraint, which
> accepts DS-form offsettable addresses, but not X-form indexed addresses.  I
> added the Z constraint so that the indexed form is accepted.

This part is fine.  Please split it out to a separate patch.

>   * config/rs6000/rs6000.md (extendsi2): Allow reg+reg indexed
>   addressing.

This should say it is changing the constraints.

>   (labelref): New insn to optimize loading a label address into
>   registers on a medium code system.

(*labelref) btw.


Segher


[PATCH], Improve PowerPC switch behavior on medium code model system

2018-07-31 Thread Michael Meissner
I noticed that the switch code on PowerPC little endian systems (with medium
code mode) did not follow the ABI in terms of page 69:

Table 2.36. Position-Independent Switch Code for Small/Medium Models
(preferred, with TOC-relative addressing)

The code we currently generate is:

.section".toc","aw"
.align 3
.LC0:
.quad   .L4
.section".text"

# ...

addis 10,2,.LC0@toc@ha
ld 10,.LC0@toc@l(10)
sldi 3,3,2
add 9,10,3
lwa 9,0(9)
add 9,9,10
mtctr 9
bctr
.L4:
.long .L2-.L4
.long .L12-.L4
.long .L11-.L4
.long .L10-.L4
.long .L9-.L4
.long .L8-.L4
.long .L7-.L4
.long .L6-.L4
.long .L5-.L4
.long .L3-.L4

While the suggested code would be something like:

addis 10,2,.L4@toc@ha
addi 10,10,.L4@toc@l
sldi 3,3,2
lwax 9,10,3
add 9,9,10
mtctr 9
bctr
.p2align 2
.align 2
.L4:
.long .L2-.L4
.long .L12-.L4
.long .L11-.L4
.long .L10-.L4
.long .L9-.L4
.long .L8-.L4
.long .L7-.L4
.long .L6-.L4
.long .L5-.L4
.long .L3-.L4

This patch adds an insn to load a LABEL_REF into a GPR.  This is needed so the
FWPROP1 pass can convert the load the of the label address from the TOC to a
direct load to a GPR.

While working on the patch, I discovered that the LWA instruction did not
support indexed loads.  This was due to it using the 'Y' constraint, which
accepts DS-form offsettable addresses, but not X-form indexed addresses.  I
added the Z constraint so that the indexed form is accepted.

I am in the middle of doing spec 2006 runs on both power8 and power9 systems
with this change.  So far after 2 runs out 3, I'm seeing several minor wins on
power9 (1-2%, perlbench, gcc, sjeng, sphinx3) and no regressions.  On power8 I
see 3 minor wins (1-3%, perlbench, sjeng, omnetpp) and 1 minor regression (1%,
povray).

I have done bootstrap builds with/without the change and there were no
regressions in the test suite.  Can I check this change into the trunk?  It is
a simple enough change for back ports, if desired.

Note, I will be on vacation for 11 days starting this Saturday.  I will not be
actively checking my mail in that time period.  If I get the approval early
enough, I can check it in.  Otherwise, somebody else can check it in if they
monitor for failure, or we can wait until I get around August 14th to check it
in.

2018-07-31  Michael Meissner  

* config/rs6000/predicates.md (label_ref_operand): New predicate
to recognize LABEL_REF.
* config/rs6000/rs6000.c (rs6000_output_addr_const_extra): Allow
LABEL_REF's inside of UNSPEC_TOCREL's.
* config/rs6000/rs6000.md (extendsi2): Allow reg+reg indexed
addressing.
(labelref): New insn to optimize loading a label address into
registers on a medium code system.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Index: gcc/config/rs6000/predicates.md
===
--- gcc/config/rs6000/predicates.md (revision 263040)
+++ gcc/config/rs6000/predicates.md (working copy)
@@ -1662,6 +1662,10 @@ (define_predicate "small_toc_ref"
   return GET_CODE (op) == UNSPEC && XINT (op, 1) == UNSPEC_TOCREL;
 })
 
+;; Match a LABEL_REF operand
+(define_predicate "label_ref_operand"
+  (match_code "label_ref"))
+
 ;; Match the first insn (addis) in fusing the combination of addis and loads to
 ;; GPR registers on power8.
 (define_predicate "fusion_gpr_addis"
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c  (revision 263040)
+++ gcc/config/rs6000/rs6000.c  (working copy)
@@ -20807,7 +20807,8 @@ rs6000_output_addr_const_extra (FILE *fi
 switch (XINT (x, 1))
   {
   case UNSPEC_TOCREL:
-   gcc_checking_assert (GET_CODE (XVECEXP (x, 0, 0)) == SYMBOL_REF
+   gcc_checking_assert ((GET_CODE (XVECEXP (x, 0, 0)) == SYMBOL_REF
+ || GET_CODE (XVECEXP (x, 0, 0)) == LABEL_REF)
 && REG_P (XVECEXP (x, 0, 1))
 && REGNO (XVECEXP (x, 0, 1)) == TOC_REGISTER);
output_addr_const (file, XVECEXP (x, 0, 0));
Index: gcc/config/rs6000/rs6000.md
===
--- gcc/config/rs6000/rs6000.md (revision 263040)
+++ gcc/config/rs6000/rs6000.md (working copy)
@@ -998,7 +998,7 @@ (define_insn "extendsi2"
 "=r, r,   wl,wu,wj,wK, wH,wr")
 
(sign_extend:EXTSI (match_operand:SI 1 "lwa_operand"
-"Y,  r,   Z, Z, r, wK, wH,?wIwH")))]
+