Re: [SH] PR 51244 - Improve T bit to reg store
Oleg Endo wrote:
> This improves some cases where the T bit is stored as all ones or all
> zeros to a GP reg.
> Tested on rev 189916 with
> make -k check RUNTESTFLAGS="--target_board=sh-sim
> \{-m2/-ml,-m2/-mb,-m2a/-mb,-m2a-single/-mb,-m4/-ml,-m4/-mb,
> -m4-single/-ml,-m4-single/-mb,-m4a-single/-ml,-m4a-single/-mb}"
>
> and no new failures.
> OK?
OK.
Regards,
kaz
Re: [SH] PR 54089 - Convert dyn left shifts to constant shifts
Oleg Endo wrote:
> In cases where dynamic shifts are available, the ashlsi3_d pattern is
> picked first and never converted back to a short sequence of constant
> shifts. Preferring short constant shift sequences over dynamic shifts
> can potentially reduce the need for an extra register to hold the shift
> count. CSiBE shows some code size increases and decreases with this
> patch. The increases seem to be due to some unlucky register
> allocations. Overall there is a small decrease (-312 bytes on the whole
> set for '-O2 -m4-single -ml -mpretend-cmove).
>
> Tested on rev 189916 with
> make -k check RUNTESTFLAGS="--target_board=sh-sim
> \{-m2/-ml,-m2/-mb,-m2a/-mb,-m2a-single/-mb,-m4/-ml,-m4/-mb,
> -m4-single/-ml,-m4-single/-mb,-m4a-single/-ml,-m4a-single/-mb}"
>
> and no new failures.
>
> OK?
OK.
Regards,
kaz
Re: [SH] PR 39423
Oleg Endo wrote:
> The attached patch adds the combine patterns as discussed in the PR.
> Tested on rev 189916 with
> make -k check RUNTESTFLAGS="--target_board=sh-sim
> \{-m2/-ml,-m2/-mb,-m2a/-mb,-m2a-single/-mb,-m4/-ml,-m4/-mb,
> -m4-single/-ml,-m4-single/-mb,-m4a-single/-ml,-m4a-single/-mb}"
>
> and no new failures.
> Test cases will follow in a separate patch.
OK.
Regards,
kaz
Re: [PATCH v2] Target-specific limits on vector alignment
> From: Ulrich Weigand > Date: Fri, 27 Jul 2012 17:24:08 +0200 > Richard (Earnshaw) has asked me to take over working on this patch now. > > I've now made the change requested above and removed the size argument. > The target is now simply asked to return the required alignment for the > given vector type. I've also added a check for the case where the > target provides both an alignment and a mode for a vector type, but > the mode actually requires bigger alignment than the type. This is > simply rejected (the target can fix this by reporting a different > type alignment or changing the mode alignment). Sounds great, IMHO. brgds, H-P
[SH] PR 54089 - Convert dyn left shifts to constant shifts
Hello,
In cases where dynamic shifts are available, the ashlsi3_d pattern is
picked first and never converted back to a short sequence of constant
shifts. Preferring short constant shift sequences over dynamic shifts
can potentially reduce the need for an extra register to hold the shift
count. CSiBE shows some code size increases and decreases with this
patch. The increases seem to be due to some unlucky register
allocations. Overall there is a small decrease (-312 bytes on the whole
set for '-O2 -m4-single -ml -mpretend-cmove).
Tested on rev 189916 with
make -k check RUNTESTFLAGS="--target_board=sh-sim
\{-m2/-ml,-m2/-mb,-m2a/-mb,-m2a-single/-mb,-m4/-ml,-m4/-mb,
-m4-single/-ml,-m4-single/-mb,-m4a-single/-ml,-m4a-single/-mb}"
and no new failures.
OK?
Cheers,
Oleg
ChangeLog:
PR target/54089
* config/sh/sh.md (ashlsi3_d): Invoke gen_shifty_op directly
instead of trying to emit ashlsi3_n.
Index: gcc/config/sh/sh.md
===
--- gcc/config/sh/sh.md (revision 189916)
+++ gcc/config/sh/sh.md (working copy)
@@ -3533,7 +3533,12 @@
}
else if (!satisfies_constraint_P27 (operands[2]))
{
- emit_insn (gen_ashlsi3_n (operands[0], operands[1], operands[2]));
+ /* This must happen before reload, otherwise the constant will be moved
+ into a register due to the "r" constraint, after which this split
+ cannot be done anymore.
+ Unfortunately the move insn will not always be eliminated. */
+ emit_move_insn (operands[0], operands[1]);
+ gen_shifty_op (ASHIFT, operands);
DONE;
}
[SH] PR 39423
Hello,
The attached patch adds the combine patterns as discussed in the PR.
Tested on rev 189916 with
make -k check RUNTESTFLAGS="--target_board=sh-sim
\{-m2/-ml,-m2/-mb,-m2a/-mb,-m2a-single/-mb,-m4/-ml,-m4/-mb,
-m4-single/-ml,-m4-single/-mb,-m4a-single/-ml,-m4a-single/-mb}"
and no new failures.
Test cases will follow in a separate patch.
Cheers,
Oleg
ChangeLog
PR target/39423
* config/gcc/sh/sh.md (*movsi_index_disp, *movhi_index_disp):
New insns.
Index: gcc/config/sh/sh.md
===
--- gcc/config/sh/sh.md (revision 189916)
+++ gcc/config/sh/sh.md (working copy)
@@ -5107,6 +5107,132 @@
"TARGET_SH1"
"sett")
+
+;; Use the combine pass to transform sequences such as
+;; mov r5,r0
+;; add #1,r0
+;; shll2 r0
+;; mov.l @(r0,r4),r0
+;; into
+;; shll2 r5
+;; add r4,r5
+;; mov.l @(4,r5),r0
+;;
+;; See also PR 39423.
+;; FIXME: Fold copy pasted patterns somehow.
+;; FIXME: Combine never tries this kind of patterns for DImode.
+(define_insn_and_split "*movsi_index_disp"
+ [(set (match_operand:SI 0 "arith_reg_dest" "=r")
+ (mem:SI
+ (plus:SI
+ (plus:SI (mult:SI (match_operand:SI 1 "arith_reg_operand" "r")
+ (match_operand:SI 2 "const_int_operand"))
+ (match_operand:SI 3 "arith_reg_operand" "r"))
+ (match_operand:SI 4 "const_int_operand"]
+ "TARGET_SH1 && sh_legitimate_index_p (SImode, operands[4], TARGET_SH2A, true)
+ && exact_log2 (INTVAL (operands[2])) > 0"
+ "#"
+ "&& can_create_pseudo_p ()"
+ [(set (match_dup 5) (ashift:SI (match_dup 1) (match_dup 2)))
+ (set (match_dup 6) (plus:SI (match_dup 5) (match_dup 3)))
+ (set (match_dup 0) (mem:SI (plus:SI (match_dup 6) (match_dup 4]
+{
+ operands[5] = gen_reg_rtx (SImode);
+ operands[6] = gen_reg_rtx (SImode);
+ operands[2] = GEN_INT (exact_log2 (INTVAL (operands[2])));
+})
+
+(define_insn_and_split "*movhi_index_disp"
+ [(set (match_operand:SI 0 "arith_reg_dest" "=r")
+ (sign_extend:SI
+ (mem:HI
+ (plus:SI
+ (plus:SI (mult:SI (match_operand:SI 1 "arith_reg_operand" "r")
+(match_operand:SI 2 "const_int_operand"))
+ (match_operand:SI 3 "arith_reg_operand" "r"))
+ (match_operand:SI 4 "const_int_operand")]
+ "TARGET_SH1 && sh_legitimate_index_p (HImode, operands[4], TARGET_SH2A, true)
+ && exact_log2 (INTVAL (operands[2])) > 0"
+ "#"
+ "&& can_create_pseudo_p ()"
+ [(set (match_dup 5) (ashift:SI (match_dup 1) (match_dup 2)))
+ (set (match_dup 6) (plus:SI (match_dup 5) (match_dup 3)))
+ (set (match_dup 0)
+ (sign_extend:SI (mem:HI (plus:SI (match_dup 6) (match_dup 4)]
+{
+ operands[5] = gen_reg_rtx (SImode);
+ operands[6] = gen_reg_rtx (SImode);
+ operands[2] = GEN_INT (exact_log2 (INTVAL (operands[2])));
+})
+
+(define_insn_and_split "*movhi_index_disp"
+ [(set (match_operand:SI 0 "arith_reg_dest" "=r")
+ (zero_extend:SI
+ (mem:HI
+ (plus:SI
+ (plus:SI (mult:SI (match_operand:SI 1 "arith_reg_operand" "r")
+(match_operand:SI 2 "const_int_operand"))
+ (match_operand:SI 3 "arith_reg_operand" "r"))
+ (match_operand:SI 4 "const_int_operand")]
+ "TARGET_SH1 && sh_legitimate_index_p (HImode, operands[4], TARGET_SH2A, true)
+ && exact_log2 (INTVAL (operands[2])) > 0"
+ "#"
+ "&& can_create_pseudo_p ()"
+ [(set (match_dup 5) (ashift:SI (match_dup 1) (match_dup 2)))
+ (set (match_dup 6) (plus:SI (match_dup 5) (match_dup 3)))
+ (set (match_dup 7)
+ (sign_extend:SI (mem:HI (plus:SI (match_dup 6) (match_dup 4)
+ (set (match_dup 0) (zero_extend:SI (match_dup 8)))]
+{
+ operands[5] = gen_reg_rtx (SImode);
+ operands[6] = gen_reg_rtx (SImode);
+ operands[7] = gen_reg_rtx (SImode);
+ operands[8] = gen_lowpart (HImode, operands[7]);
+ operands[2] = GEN_INT (exact_log2 (INTVAL (operands[2])));
+})
+
+(define_insn_and_split "*movsi_index_disp"
+ [(set (mem:SI
+ (plus:SI
+ (plus:SI (mult:SI (match_operand:SI 1 "arith_reg_operand" "r")
+ (match_operand:SI 2 "const_int_operand"))
+ (match_operand:SI 3 "arith_reg_operand" "r"))
+ (match_operand:SI 4 "const_int_operand")))
+ (match_operand:SI 0 "arith_reg_operand" "r"))]
+ "TARGET_SH1 && sh_legitimate_index_p (SImode, operands[4], TARGET_SH2A, true)
+ && exact_log2 (INTVAL (operands[2])) > 0"
+ "#"
+ "&& can_create_pseudo_p ()"
+ [(set (match_dup 5) (ashift:SI (match_dup 1) (match_dup 2)))
+ (set (match_dup 6) (plus:SI (match_dup 5) (match_dup 3)))
+ (set (mem:SI (plus:SI (match_dup 6) (match_dup 4))) (match_dup 0))]
+{
+ operands[5] = gen_reg_rtx (SImode);
+ operands[6] = gen_reg_rtx (SImode);
+ operands[2] = GEN_INT (exact_log2 (INTVAL (operands[2])));
+})
+
+(define_insn_and_split "*movhi_index_disp"
+ [(set (mem:HI
+ (plus:SI
+ (plus:SI (mult:SI (match_operand:SI 1 "arith_reg_operand" "r")
+ (match_operand:SI 2 "const_int_operand"))
+ (match_operand:SI 3 "arith_reg_operand" "r"))
+ (match_operand:SI 4 "const_int_operand")))
+ (match
[SH] PR 54089 - Convert dyn left shifts to constant shifts
Hello,
In cases where dynamic shifts are available, the ashlsi3_d pattern is
picked first and never converted back to a short sequence of constant
shifts. Preferring short constant shift sequences over dynamic shifts
can potentially reduce the need for an extra register to hold the shift
count. CSiBE shows some code size increases and decreases with this
patch. The increases seem to be due to some unlucky register
allocations. Overall there is a small decrease (-312 bytes on the whole
set for '-O2 -m4-single -ml -mpretend-cmove).
OK?
Cheers,
Oleg
ChangeLog:
PR target/54089
* config/sh/sh.md (ashlsi3_d): Invoke gen_shifty_op directly
instead of trying to emit ashlsi3_n.
Index: gcc/config/sh/sh.md
===
--- gcc/config/sh/sh.md (revision 189916)
+++ gcc/config/sh/sh.md (working copy)
@@ -3533,7 +3533,12 @@
}
else if (!satisfies_constraint_P27 (operands[2]))
{
- emit_insn (gen_ashlsi3_n (operands[0], operands[1], operands[2]));
+ /* This must happen before reload, otherwise the constant will be moved
+ into a register due to the "r" constraint, after which this split
+ cannot be done anymore.
+ Unfortunately the move insn will not always be eliminated. */
+ emit_move_insn (operands[0], operands[1]);
+ gen_shifty_op (ASHIFT, operands);
DONE;
}
[SH] PR 51244 - Improve T bit to reg store
Hello,
This improves some cases where the T bit is stored as all ones or all
zeros to a GP reg.
Tested on rev 189916 with
make -k check RUNTESTFLAGS="--target_board=sh-sim
\{-m2/-ml,-m2/-mb,-m2a/-mb,-m2a-single/-mb,-m4/-ml,-m4/-mb,
-m4-single/-ml,-m4-single/-mb,-m4a-single/-ml,-m4a-single/-mb}"
and no new failures.
OK?
Cheers,
Oleg
ChangeLog:
PR target/51244
* config/sh/sh.md (mov_neg_si_t): Move to Scc instructions
section.
Use t_reg_operand predicate. Add split for negated case.
(ashrsi2_31): Pass get_t_reg_rtx to gen_mov_neg_si_t.
* config/sh/sh.c (expand_ashiftrt): Likewise.
testsuite/ChangeLog:
PR target/51244
* gcc.target/sh/pr51244-4.c: New.
Index: gcc/config/sh/sh.c
===
--- gcc/config/sh/sh.c (revision 189917)
+++ gcc/config/sh/sh.c (working copy)
@@ -3481,7 +3481,7 @@
{
emit_insn (gen_cmpgtsi_t (force_reg (SImode, CONST0_RTX (SImode)),
operands[1]));
- emit_insn (gen_mov_neg_si_t (operands[0]));
+ emit_insn (gen_mov_neg_si_t (operands[0], get_t_reg_rtx ()));
return true;
}
emit_insn (gen_ashrsi2_31 (operands[0], operands[1]));
Index: gcc/config/sh/sh.md
===
--- gcc/config/sh/sh.md (revision 189916)
+++ gcc/config/sh/sh.md (working copy)
@@ -1584,15 +1584,6 @@
"subc %2,%0"
[(set_attr "type" "arith")])
-;; life_analysis thinks rn is live before subc rn,rn, so make a special
-;; pattern for this case. This helps multimedia applications that compute
-;; the sum of absolute differences.
-(define_insn "mov_neg_si_t"
- [(set (match_operand:SI 0 "arith_reg_dest" "=r") (neg:SI (reg:SI T_REG)))]
- "TARGET_SH1"
- "subc %0,%0"
- [(set_attr "type" "arith")])
-
(define_insn "*subsi3_internal"
[(set (match_operand:SI 0 "arith_reg_dest" "=r")
(minus:SI (match_operand:SI 1 "arith_reg_operand" "0")
@@ -3797,7 +3788,7 @@
[(const_int 0)]
{
emit_insn (gen_ashlsi_c (operands[0], operands[1]));
- emit_insn (gen_mov_neg_si_t (copy_rtx (operands[0])));
+ emit_insn (gen_mov_neg_si_t (operands[0], get_t_reg_rtx ()));
DONE;
})
@@ -9704,6 +9695,25 @@
""
[(const_int 0)])
+;; Store T bit as all zeros or ones in a reg.
+(define_insn "mov_neg_si_t"
+ [(set (match_operand:SI 0 "arith_reg_dest" "=r")
+ (neg:SI (match_operand 1 "t_reg_operand" "")))]
+ "TARGET_SH1"
+ "subc %0,%0"
+ [(set_attr "type" "arith")])
+
+;; Store negated T bit as all zeros or ones in a reg.
+;; Use the following sequence:
+;; subc Rn,Rn ! Rn = Rn - Rn - T; T = T
+;; not Rn,Rn ! Rn = 0 - Rn
+(define_split
+ [(set (match_operand:SI 0 "arith_reg_dest" "")
+ (neg:SI (match_operand 1 "negt_reg_operand" "")))]
+ "TARGET_SH1"
+ [(set (match_dup 0) (neg:SI (reg:SI T_REG)))
+ (set (match_dup 0) (not:SI (match_dup 0)))])
+
;; The *movtt pattern eliminates redundant T bit to T bit moves / tests.
(define_insn_and_split "*movtt"
[(set (reg:SI T_REG)
Index: gcc/testsuite/gcc.target/sh/pr51244-4.c
===
--- gcc/testsuite/gcc.target/sh/pr51244-4.c (revision 0)
+++ gcc/testsuite/gcc.target/sh/pr51244-4.c (revision 0)
@@ -0,0 +1,19 @@
+/* Check that storing the (negated) T bit as all ones or zeros in a reg
+ uses the subc instruction. On SH2A a sequence with the movrt instruction
+ is also OK instead of subc. */
+/* { dg-do compile { target "sh*-*-*" } } */
+/* { dg-options "-O1 -mbranch-cost=2" } */
+/* { dg-skip-if "" { "sh*-*-*" } { "-m5*"} { "" } } */
+/* { dg-final { scan-assembler-not "movt|tst|negc" } } */
+/* { dg-final { scan-assembler "subc|movrt|neg|not" } } */
+
+int test_00 (int x, int y)
+{
+ return x != y ? -1 : 0;
+}
+
+int test_01 (int x, int y)
+{
+ return x == y ? -1 : 0;
+}
+
Re: cleanup of CONST_DOUBLE.
I will add the comment. However, my hope is that will be short lived knowledge, but i am a couple of patches away from that. kenny On 07/29/2012 05:31 PM, Steven Bosscher wrote: On Sun, Jul 29, 2012 at 6:11 PM, Kenneth Zadeck wrote: * rtl.h (CONST_DOUBLE_AS_INT_P, CONST_DOUBLE_AS_FLOAT_P): New macros. Hello Kenny, Thanks for doing this! Can you please also update the comment before CONST_DOUBLE in rtl.def? According to rtl.def CONST_DOUBLEs are only for floating point constants. (I only found out that CONST_DOUBLEs can be ints a few days ago :-) Ciao! Steven
Re: cleanup of CONST_DOUBLE.
On Sun, Jul 29, 2012 at 6:11 PM, Kenneth Zadeck wrote: > * rtl.h (CONST_DOUBLE_AS_INT_P, CONST_DOUBLE_AS_FLOAT_P): New > macros. Hello Kenny, Thanks for doing this! Can you please also update the comment before CONST_DOUBLE in rtl.def? According to rtl.def CONST_DOUBLEs are only for floating point constants. (I only found out that CONST_DOUBLEs can be ints a few days ago :-) Ciao! Steven
[CFT] s390: Convert from sync to atomic optabs
Tested only as far as cross-compile. I had a browse through
objdump of libatomic for a brief sanity check.
Can you please test on real hw and report back?
r~
---
gcc/config/s390/s390-protos.h |3 +-
gcc/config/s390/s390.c| 90 +-
gcc/config/s390/s390.md | 373 -
3 files changed, 308 insertions(+), 158 deletions(-)
diff --git a/gcc/config/s390/s390-protos.h b/gcc/config/s390/s390-protos.h
index 4f1eb42..79673d6 100644
--- a/gcc/config/s390/s390-protos.h
+++ b/gcc/config/s390/s390-protos.h
@@ -85,7 +85,8 @@ extern void s390_expand_setmem (rtx, rtx, rtx);
extern bool s390_expand_cmpmem (rtx, rtx, rtx, rtx);
extern bool s390_expand_addcc (enum rtx_code, rtx, rtx, rtx, rtx, rtx);
extern bool s390_expand_insv (rtx, rtx, rtx, rtx);
-extern void s390_expand_cs_hqi (enum machine_mode, rtx, rtx, rtx, rtx);
+extern void s390_expand_cs_hqi (enum machine_mode, rtx, rtx, rtx,
+ rtx, rtx, bool);
extern void s390_expand_atomic (enum machine_mode, enum rtx_code,
rtx, rtx, rtx, bool);
extern rtx s390_return_addr_rtx (int, rtx);
diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index f72f49f..0685c6c 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -896,10 +896,12 @@ s390_emit_compare (enum rtx_code code, rtx op0, rtx op1)
conditional branch testing the result. */
static rtx
-s390_emit_compare_and_swap (enum rtx_code code, rtx old, rtx mem, rtx cmp, rtx
new_rtx)
+s390_emit_compare_and_swap (enum rtx_code code, rtx old, rtx mem,
+ rtx cmp, rtx new_rtx)
{
- emit_insn (gen_sync_compare_and_swapsi (old, mem, cmp, new_rtx));
- return s390_emit_compare (code, gen_rtx_REG (CCZ1mode, CC_REGNUM),
const0_rtx);
+ emit_insn (gen_atomic_compare_and_swapsi_internal (old, mem, cmp, new_rtx));
+ return s390_emit_compare (code, gen_rtx_REG (CCZ1mode, CC_REGNUM),
+ const0_rtx);
}
/* Emit a jump instruction to TARGET. If COND is NULL_RTX, emit an
@@ -4720,79 +4722,79 @@ init_alignment_context (struct alignment_context *ac,
rtx mem,
}
/* Expand an atomic compare and swap operation for HImode and QImode. MEM is
- the memory location, CMP the old value to compare MEM with and NEW_RTX the
value
- to set if CMP == MEM.
- CMP is never in memory for compare_and_swap_cc because
- expand_bool_compare_and_swap puts it into a register for later compare. */
+ the memory location, CMP the old value to compare MEM with and NEW_RTX the
+ value to set if CMP == MEM. */
void
-s390_expand_cs_hqi (enum machine_mode mode, rtx target, rtx mem, rtx cmp, rtx
new_rtx)
+s390_expand_cs_hqi (enum machine_mode mode, rtx btarget, rtx vtarget, rtx mem,
+ rtx cmp, rtx new_rtx, bool is_weak)
{
struct alignment_context ac;
rtx cmpv, newv, val, resv, cc;
rtx res = gen_reg_rtx (SImode);
- rtx csloop = gen_label_rtx ();
- rtx csend = gen_label_rtx ();
+ rtx csloop = NULL, csend = NULL;
- gcc_assert (register_operand (target, VOIDmode));
+ gcc_assert (register_operand (vtarget, VOIDmode));
gcc_assert (MEM_P (mem));
init_alignment_context (&ac, mem, mode);
/* Shift the values to the correct bit positions. */
- if (!(ac.aligned && MEM_P (cmp)))
-cmp = s390_expand_mask_and_shift (cmp, mode, ac.shift);
- if (!(ac.aligned && MEM_P (new_rtx)))
-new_rtx = s390_expand_mask_and_shift (new_rtx, mode, ac.shift);
+ cmp = s390_expand_mask_and_shift (cmp, mode, ac.shift);
+ new_rtx = s390_expand_mask_and_shift (new_rtx, mode, ac.shift);
/* Load full word. Subsequent loads are performed by CS. */
val = expand_simple_binop (SImode, AND, ac.memsi, ac.modemaski,
NULL_RTX, 1, OPTAB_DIRECT);
/* Start CS loop. */
- emit_label (csloop);
+ if (!is_weak)
+{
+ /* Begin assuming success. */
+ emit_move_insn (btarget, const1_rtx);
+
+ csloop = gen_label_rtx ();
+ csend = gen_label_rtx ();
+ emit_label (csloop);
+}
+
/* val = "00..0"
* cmp = "00..000..0"
* new = "00..000..0"
*/
/* Patch cmp and new with val at correct position. */
- if (ac.aligned && MEM_P (cmp))
+ cmpv = force_reg (SImode, expand_simple_binop (SImode, IOR, cmp, val,
+NULL_RTX, 1, OPTAB_DIRECT));
+ newv = force_reg (SImode, expand_simple_binop (SImode, IOR, new_rtx, val,
+NULL_RTX, 1, OPTAB_DIRECT));
+
+ if (is_weak)
{
- cmpv = force_reg (SImode, val);
- store_bit_field (cmpv, GET_MODE_BITSIZE (mode), 0,
- 0, 0, SImode, cmp);
+ cc = s390_emit_compare_and_swap (NE, res, ac.memsi, cmpv, newv);
+ emit_insn (gen_cstorecc4 (btarget, cc, XEXP (cc, 0), XEXP (cc, 1)));
}
else
-cmpv = force_reg (SImode, expand_simple_binop (SImode, IOR, cmp, val,
-
Re: PR 54075 Restore 4.6 growth factor
On 07/29/2012 07:38 PM, Jonathan Wakely wrote: Yes please. The point is to allow people to review and comment before the patch is applied, and some people only subscribe to gcc-patches not libstdc++. I don't have a strong opinion, but I must say that I don't understand why those people don't subscribe to libstdc++: since we *do* have a separate mailing list, IMO if they are interested they should. On the other hand, if they *really* don't care (eg, some people don't even like C++ in general ;) sending all the traffic to gcc-patches too may seem a mild form of spamming. But, as I said, I don't have a strong opinion, and personally I'm not terribly consistent ;) Paolo.
Re: PR 54075 Restore 4.6 growth factor
On 29 July 2012 18:15, François Dumont wrote: > Patch applied. I usually CC to gcc-patches when I signal that it has been > applied. Should I send it all my patch proposals ? Yes please. The point is to allow people to review and comment before the patch is applied, and some people only subscribe to gcc-patches not libstdc++. The gcc-cvs and libstdc++-cvs lists provide a record of patches that have been applied.
Re: PR 54075 Restore 4.6 growth factor
Patch applied. I usually CC to gcc-patches when I signal that it has
been applied. Should I send it all my patch proposals ?
François
On 07/28/2012 11:18 PM, Jonathan Wakely wrote:
Please remember to CC gcc-patches too.
On 28 July 2012 21:49, François Dumont wrote:
Hi
Here is the patch to restore the 4.6 growth factor of 2. I prefer to
validate the restored behavior by adding a performance test. Without the
patch the result was:
unordered_set.cc unordered_set 1000 insertions 403r 329u
73s 402825280mem0pf
after the patch:
unordered_set.cc unordered_set 1000 insertions 112r 86u
25s 402825104mem0pf
It validates the 3x times performance hint.
Tested under Linux x86_64.
2012-07-28 François Dumont
PR libstdc++/54075
* include/bits/hashtable_policy.h
(_Prime_rehash_policy::_M_next_bkt): Add a growth factor set to 2
to boost growth in the number of buckets.
* testsuite/performance/23_containers/insert/unordered_set.cc: New.
Even if it is not a Standard conformity issue I think we can apply it to the
4.7 branch too.
Yes, it's a performance regression, so this is OK for trunk and 4.7, thanks.
Index: include/bits/hashtable_policy.h
===
--- include/bits/hashtable_policy.h (revision 189893)
+++ include/bits/hashtable_policy.h (working copy)
@@ -395,6 +395,8 @@
enum { _S_n_primes = sizeof(unsigned long) != 8 ? 256 : 256 + 48 };
+static const std::size_t _S_growth_factor = 2;
+
float_M_max_load_factor;
mutable std::size_t _M_prev_resize;
mutable std::size_t _M_next_resize;
@@ -415,28 +417,27 @@
static const unsigned char __fast_bkt[12]
= { 2, 2, 2, 3, 5, 5, 7, 7, 11, 11, 11, 11 };
-if (__n <= 11)
+const std::size_t __grown_n = __n * _S_growth_factor;
+if (__grown_n <= 11)
{
_M_prev_resize = 0;
_M_next_resize
- = __builtin_ceil(__fast_bkt[__n] * (long double)_M_max_load_factor);
- return __fast_bkt[__n];
+ = __builtin_ceil(__fast_bkt[__grown_n]
+ * (long double)_M_max_load_factor);
+ return __fast_bkt[__grown_n];
}
-const unsigned long* __p
- = std::lower_bound(__prime_list + 5, __prime_list + _S_n_primes, __n);
+const unsigned long* __next_bkt
+ = std::lower_bound(__prime_list + 5, __prime_list + _S_n_primes,
+ __grown_n);
+const unsigned long* __prev_bkt
+ = std::lower_bound(__prime_list + 1, __next_bkt, __n / _S_growth_factor);
-// Shrink will take place only if the number of elements is small enough
-// so that the prime number 2 steps before __p is large enough to still
-// conform to the max load factor:
_M_prev_resize
- = __builtin_floor(*(__p - 2) * (long double)_M_max_load_factor);
-
-// Let's guaranty that a minimal grow step of 11 is used
-if (*__p - __n < 11)
- __p = std::lower_bound(__p, __prime_list + _S_n_primes, __n + 11);
-_M_next_resize = __builtin_ceil(*__p * (long double)_M_max_load_factor);
-return *__p;
+ = __builtin_floor(*(__prev_bkt - 1) * (long double)_M_max_load_factor);
+_M_next_resize
+ = __builtin_ceil(*__next_bkt * (long double)_M_max_load_factor);
+return *__next_bkt;
}
// Return the smallest prime p such that alpha p >= n, where alpha
Index: testsuite/performance/23_containers/insert/unordered_set.cc
===
--- testsuite/performance/23_containers/insert/unordered_set.cc (revision 0)
+++ testsuite/performance/23_containers/insert/unordered_set.cc (revision 0)
@@ -0,0 +1,42 @@
+// Copyright (C) 2012 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+//
Re: cleanup of CONST_DOUBLE.
sorry, will fix all of this. thanks. On 07/29/2012 12:33 PM, Richard Sandiford wrote: Kenneth Zadeck writes: Given that Richard Sandiford advised on all of the non trivial changes, I am going to check this patch in in the next few days unless i hear some comments otherwise. TBH I'd only looked at the ones you flagged. This time... diff -puNr '--exclude=.git' '--exclude=.svn' gccBaseline/gcc/cfgexpand.c gccWide/gcc/cfgexpand.c --- gccBaseline/gcc/cfgexpand.c 2012-07-22 16:55:01.235983016 -0400 +++ gccWide/gcc/cfgexpand.c 2012-07-25 19:42:16.456201001 -0400 @@ -3634,7 +3634,7 @@ expand_debug_locations (void) || (GET_MODE (val) == VOIDmode && (CONST_INT_P (val) || GET_CODE (val) == CONST_FIXED - || GET_CODE (val) == CONST_DOUBLE + || CONST_DOUBLE_P (val) || GET_CODE (val) == LABEL_REF))); should be CONST_DOUBLE_AS_INT_P (because we know it's VOIDmode already). - if ((CONSTANT_P (op) && !CONST_INT_P (op) - && (GET_CODE (op) != CONST_DOUBLE || GET_MODE (op) != VOIDmode)) + if ((CONSTANT_P (op) && !CONST_INT_P (op) && !CONST_DOUBLE_AS_INT_P (op)) Long line (keep on two). +/* Predicate yielding true iff X is an rtx for a double-int. */ +#define CONST_DOUBLE_AS_INT_P(X) (GET_CODE (X) == CONST_DOUBLE && GET_MODE (X) == VOIDmode) + +/* Predicate yielding true iff X is an rtx for a double-int. */ +#define CONST_DOUBLE_AS_FLOAT_P(X) (GET_CODE (X) == CONST_DOUBLE && GET_MODE (X) != VOIDmode) + Long lines. Should be: #define CONST_DOUBLE_AS_INT_P(X) \ (GET_CODE (X) == CONST_DOUBLE && GET_MODE (X) == VOIDmode) etc. - && (CONST_INT_P (XEXP (op0, 1)) - || GET_CODE (XEXP (op0, 1)) == CONST_DOUBLE) + && (CONST_INT_P (XEXP (op0, 1)) || CONST_DOUBLE_AS_INT_P (XEXP (op0, 1))) Long line (keep on two). - && (CONST_INT_P (XEXP (op0, 1)) - || GET_CODE (XEXP (op0, 1)) == CONST_DOUBLE) + && (CONST_INT_P (XEXP (op0, 1)) || CONST_DOUBLE_AS_INT_P (XEXP (op0, 1))) Same. - && (CONST_INT_P (op1) - || GET_CODE (op1) == CONST_DOUBLE) - && (CONST_INT_P (XEXP (op0, 1)) - || GET_CODE (XEXP (op0, 1)) == CONST_DOUBLE)) + && (CONST_INT_P (op1) || CONST_DOUBLE_AS_INT_P (op1)) + && (CONST_INT_P (XEXP (op0, 1)) || CONST_DOUBLE_AS_INT_P (XEXP (op0, 1 Same. Looks good otherwise. Richard
Re: cleanup of CONST_DOUBLE.
> Given that Richard Sandiford advised on all of the non trivial changes, > I am going to check this patch in in the next few days unless i hear > some comments otherwise. This patch has been fully tested on the x86-64. Pasto in the rtl.h change. Please also avoid the long lines in there. -- Eric Botcazou
Re: cleanup of CONST_DOUBLE.
Kenneth Zadeck writes: > Given that Richard Sandiford advised on all of the non trivial changes, > I am going to check this patch in in the next few days unless i hear > some comments otherwise. TBH I'd only looked at the ones you flagged. This time... > diff -puNr '--exclude=.git' '--exclude=.svn' gccBaseline/gcc/cfgexpand.c > gccWide/gcc/cfgexpand.c > --- gccBaseline/gcc/cfgexpand.c 2012-07-22 16:55:01.235983016 -0400 > +++ gccWide/gcc/cfgexpand.c 2012-07-25 19:42:16.456201001 -0400 > @@ -3634,7 +3634,7 @@ expand_debug_locations (void) > || (GET_MODE (val) == VOIDmode > && (CONST_INT_P (val) > || GET_CODE (val) == CONST_FIXED > - || GET_CODE (val) == CONST_DOUBLE > + || CONST_DOUBLE_P (val) > || GET_CODE (val) == LABEL_REF))); should be CONST_DOUBLE_AS_INT_P (because we know it's VOIDmode already). > - if ((CONSTANT_P (op) && !CONST_INT_P (op) > -&& (GET_CODE (op) != CONST_DOUBLE || GET_MODE (op) != VOIDmode)) > + if ((CONSTANT_P (op) && !CONST_INT_P (op) && !CONST_DOUBLE_AS_INT_P > (op)) Long line (keep on two). > +/* Predicate yielding true iff X is an rtx for a double-int. */ > +#define CONST_DOUBLE_AS_INT_P(X) (GET_CODE (X) == CONST_DOUBLE && GET_MODE > (X) == VOIDmode) > + > +/* Predicate yielding true iff X is an rtx for a double-int. */ > +#define CONST_DOUBLE_AS_FLOAT_P(X) (GET_CODE (X) == CONST_DOUBLE && GET_MODE > (X) != VOIDmode) > + Long lines. Should be: #define CONST_DOUBLE_AS_INT_P(X) \ (GET_CODE (X) == CONST_DOUBLE && GET_MODE (X) == VOIDmode) etc. > - && (CONST_INT_P (XEXP (op0, 1)) > - || GET_CODE (XEXP (op0, 1)) == CONST_DOUBLE) > + && (CONST_INT_P (XEXP (op0, 1)) || CONST_DOUBLE_AS_INT_P (XEXP (op0, > 1))) Long line (keep on two). > - && (CONST_INT_P (XEXP (op0, 1)) > - || GET_CODE (XEXP (op0, 1)) == CONST_DOUBLE) > + && (CONST_INT_P (XEXP (op0, 1)) || CONST_DOUBLE_AS_INT_P (XEXP (op0, > 1))) Same. > - && (CONST_INT_P (op1) > - || GET_CODE (op1) == CONST_DOUBLE) > - && (CONST_INT_P (XEXP (op0, 1)) > - || GET_CODE (XEXP (op0, 1)) == CONST_DOUBLE)) > + && (CONST_INT_P (op1) || CONST_DOUBLE_AS_INT_P (op1)) > + && (CONST_INT_P (XEXP (op0, 1)) || CONST_DOUBLE_AS_INT_P (XEXP (op0, > 1 Same. Looks good otherwise. Richard
cleanup of CONST_DOUBLE.
This patch is a minor, mostly mechanical cleanup of CONST_DOUBLE. It
wraps all of the checks of CONST_DOUBLE in one of three macros:
CONST_DOUBLE_AS_INT_P, CONST_DOUBLE_AS_FLOAT_P, or CONST_DOUBLE_P is it
used for both. There were some non obvious changes that Richard
Sandiford told me the how he wanted them changed, like the test in first
frag or rtlanal.c that he had me delete the test altogether.
There are also some head scratchers like the third frag in
simplify_binary_operation_1.
The test for this could have never been true since CONST_DOUBLEs only
have a non void mode
if they are floats. This test was removed.
|| GET_MODE_CLASS (GET_MODE (trueop1)) == MODE_INT)
Given that Richard Sandiford advised on all of the non trivial changes,
I am going to check this patch in in the next few days unless i hear
some comments otherwise. This patch has been fully tested on the x86-64.
2012-07-29 Kenneth Zadeck
* cfgexpand.c (expand_debug_locations): Encapsulate test for
CONST_DOUBLE in macro.
* combine.c (try_combine, gen_lowpart_for_combine): Ditto.
* cprop.c (implicit_set_cond_p): Ditto.
* cselib.c (rtx_equal_for_cselib_1): Ditto.
* expmed.c (expand_mult): Ditto.
* expr.c (convert_modes): Ditto.
* ira-costs.c (record_reg_classes): Ditto.
* ira-lives.c (single_reg_class): Ditto.
* optabs.c (expand_copysign_absneg, expand_copysign): Ditto.
* print-rtl.c (print_rtx): Ditto.
* recog.c (simplify_while_replacing, const_double_operand,
asm_operand_ok, constrain_operands): Ditto.
* reg-stack.c (subst_stack_regs_pat): Ditto.
* reload.c (find_reloads, find_equiv_reg): Ditto.
* rtlanal.c (replace_rtx): Remove test.
* rtlanal.c (constant_pool_constant_p, split_double): Encapsulate
test for
CONST_DOUBLE in macro.
* simplify-rtx.c (mode_signbit_p, avoid_constant_pool_reference,
simplify_unary_operation_1, simplify_const_unary_operation,
simplify_binary_operation_1, simplify_const_binary_operation,
simplify_relational_operation_1,
simplify_const_relational_operations,
implify_subreg): Ditto.
* varasm.c (output_constant_pool_2): Ditto.
* rtl.h (CONST_DOUBLE_AS_INT_P, CONST_DOUBLE_AS_FLOAT_P): New
macros.
diff -puNr '--exclude=.git' '--exclude=.svn' gccBaseline/gcc/cfgexpand.c gccWide/gcc/cfgexpand.c
--- gccBaseline/gcc/cfgexpand.c 2012-07-22 16:55:01.235983016 -0400
+++ gccWide/gcc/cfgexpand.c 2012-07-25 19:42:16.456201001 -0400
@@ -3634,7 +3634,7 @@ expand_debug_locations (void)
|| (GET_MODE (val) == VOIDmode
&& (CONST_INT_P (val)
|| GET_CODE (val) == CONST_FIXED
-|| GET_CODE (val) == CONST_DOUBLE
+|| CONST_DOUBLE_P (val)
|| GET_CODE (val) == LABEL_REF)));
}
diff -puNr '--exclude=.git' '--exclude=.svn' gccBaseline/gcc/combine.c gccWide/gcc/combine.c
--- gccBaseline/gcc/combine.c 2012-07-22 16:55:01.243982920 -0400
+++ gccWide/gcc/combine.c 2012-07-28 16:40:26.975209932 -0400
@@ -2775,10 +2775,10 @@ try_combine (rtx i3, rtx i2, rtx i1, rtx
if (i1 == 0
&& (temp = single_set (i2)) != 0
&& (CONST_INT_P (SET_SRC (temp))
- || GET_CODE (SET_SRC (temp)) == CONST_DOUBLE)
+ || CONST_DOUBLE_AS_INT_P (SET_SRC (temp)))
&& GET_CODE (PATTERN (i3)) == SET
&& (CONST_INT_P (SET_SRC (PATTERN (i3)))
- || GET_CODE (SET_SRC (PATTERN (i3))) == CONST_DOUBLE)
+ || CONST_DOUBLE_AS_INT_P (SET_SRC (PATTERN (i3
&& reg_subword_p (SET_DEST (PATTERN (i3)), SET_DEST (temp)))
{
rtx dest = SET_DEST (PATTERN (i3));
@@ -5253,8 +5253,7 @@ subst (rtx x, rtx from, rtx to, int in_d
return new_rtx;
if (GET_CODE (x) == SUBREG
- && (CONST_INT_P (new_rtx)
- || GET_CODE (new_rtx) == CONST_DOUBLE))
+ && (CONST_INT_P (new_rtx) || CONST_DOUBLE_AS_INT_P (new_rtx)))
{
enum machine_mode mode = GET_MODE (x);
@@ -7282,8 +7281,7 @@ make_extraction (enum machine_mode mode,
if (mode == tmode)
return new_rtx;
- if (CONST_INT_P (new_rtx)
- || GET_CODE (new_rtx) == CONST_DOUBLE)
+ if (CONST_INT_P (new_rtx) || CONST_DOUBLE_AS_INT_P (new_rtx))
return simplify_unary_operation (unsignedp ? ZERO_EXTEND : SIGN_EXTEND,
mode, new_rtx, tmode);
@@ -10794,9 +10792,7 @@ gen_lowpart_for_combine (enum machine_mo
/* We can only support MODE being wider than a word if X is a
constant integer or has a mode the same size. */
if (GET_MODE_SIZE (omode) > UNITS_PER_WORD
- && ! ((imode == VOIDmode
- && (CONST_INT_P (x)
- || GET_CODE (x) == CONST_DOUBLE))
+ && ! ((CONST_INT_P (x) || CONST_DOUBLE_AS_INT_P (x))
|| isize == osize))
goto fail;
diff -puNr '--exclude=.git' '--exclude=.svn' gccBaseline/gcc/cprop.c gccWide/gcc/cprop.c
--- gccBaseline/gcc/cprop.c 2012-07-22 16:55:01.247982871 -0400
+++ gccWide/gcc/cprop.c 2012-07-25 19:52:57.064313634 -0400
@@ -1327,7 +1327,7 @@ implicit_set_cond_p (const_rtx cond)
the optimization can't be performed. */
/* ???
[Patch, Fortran, F03] PR 51081: Proc-pointer assignment: Rejects valid internal proc
Hi all, here is a second patch for PR 51081, which fixes the rejection of the valid test case in comment 1. The problem was that the checking for intrinsics came too early (in gfc_match_rvalue), so that INT2 was marked as intrinsic, before it was known that there is a contained procedure which shadows the intrinsic. I have moved the check to resolution stage (resolve_symbol), where it is done for all flavorless symbols. One has to be a bit careful here with variable names which collide with intrinsic procedures. Initially there were a couple of testsuite failures due to this, most of which I have killed by checking if the symbol's type is known. However, one testsuite failure was left (gfortran.fortran-torture/compile/pr39937.f), for which I think it is basically impossible to decide that 'SCALE' is meant to be a variable name, and not an intrinsic procedure. So my interpretation would be that SCALE refers to the intrinsic here, which makes the test case invalid. I changed the name of the variable to make it valid. That test case is apparently a reduction of a larger code (from SPEC CPU 2006), which hopefully will not fail due to this patch (unfortunately I cannot check this). The patch was regtested on x86_64-unknown-linux-gnu. Ok for trunk? Cheers, Janus 2012-07-29 Janus Weil PR fortran/51081 * primary.c (gfc_match_rvalue): Check for intrinsics came too early. * resolve.c (resolve_symbol): Check if flavorless symbols could be intrinsic. 2012-07-29 Janus Weil PR fortran/51081 * gfortran.fortran-torture/compile/pr39937.f: Modified. * gfortran.dg/proc_ptr_37.f90: New. pr51081_part2.diff Description: Binary data proc_ptr_37.f90 Description: Binary data
[patch, fortran] Fix PR 54033, problems with -I, with test cases
Hello world,
here is an updated patch for PR 54033, this time with test cases.
Thanks to Janis for pointing me in the right direction with these.
Regression-tested. OK for trunk?
Thomas
2012-07-29 Thomas König
PR fortran/54033
* scanner.c (add_path_to_list): Emit warning if an error occurs
for an include path, if it is not present or if it is not a
directory. Do not add the path in these cases.
2012-07-29 Thomas König
PR fortran/54033
* gfortran.dg/include_6.f90: New test case.
* gfortran.dg/include_7.f90: New test case.
* gfortran.dg/include_3.f90: Add dg-warning for missing directory.
! { dg-do compile }
! { dg-options "-I gfortran.log" }
! { dg-warning "is not a directory" "" { target *-*-* } 0 }
end
! { dg-do compile }
! { dg-options "-I nothere" }
! { dg-warning "Nonexistent include directory" "missing directory" { target *-*-* } 0 }
end
Index: fortran/scanner.c
===
--- fortran/scanner.c (Revision 189754)
+++ fortran/scanner.c (Arbeitskopie)
@@ -311,12 +311,30 @@ add_path_to_list (gfc_directorylist **list, const
{
gfc_directorylist *dir;
const char *p;
-
+ struct stat st;
+
p = path;
while (*p == ' ' || *p == '\t') /* someone might do "-I include" */
if (*p++ == '\0')
return;
+ if (stat (p, &st))
+{
+ if (errno != ENOENT)
+ gfc_warning_now ("Include directory \"%s\": %s", path,
+ xstrerror(errno));
+ else
+ /* FIXME: Also support -Wmissing-include-dirs. */
+ gfc_warning_now ("Nonexistent include directory \"%s\"", path);
+ return;
+}
+
+ else if (!S_ISDIR (st.st_mode))
+{
+ gfc_warning_now ("\"%s\" is not a directory", path);
+ return;
+}
+
if (head || *list == NULL)
{
dir = XCNEW (gfc_directorylist);
Index: testsuite/gfortran.dg/include_3.f95
===
--- testsuite/gfortran.dg/include_3.f95 (Revision 189754)
+++ testsuite/gfortran.dg/include_3.f95 (Arbeitskopie)
@@ -24,3 +24,4 @@ end function
! { dg-do compile }
! { dg-options "-fpreprocessed -g3" }
+! { dg-warning "Nonexistent include directory" "missing directory" { target *-*-* } 0 }
Re: ping ancient MIPS: missed optimization patch
Andrew Pinski writes: > On Fri, Jul 27, 2012 at 12:15 PM, Sandra Loosemore > wrote: >> Richard, >> >> This ancient patch to tweak mips_legitimize_address >> >> http://gcc.gnu.org/ml/gcc/2008-11/msg00294.html >> >> seems to never have been applied. Do you have any idea whether this is >> still a useful change? The test case given in the first message in that >> thread no longer reproduces with a recent mainline build > > I think http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33699#c9 was the > change which fixes the original testcase. Yeah. Adam's patch was a much better fix than my quick hack above. It exposed the requirements to the rtl optimisers rather than hiding them in the target code. Richard
