* RFA: Fix for compile/20010903-1.c regressions
@ 2002-07-23 9:24 Joern Rennecke
2002-07-29 14:51 ` Richard Henderson
0 siblings, 1 reply; 6+ messages in thread
From: Joern Rennecke @ 2002-07-23 9:24 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 107 bytes --]
--
--------------------------
SuperH
2430 Aztec West / Almondsbury / BRISTOL / BS32 4AQ
T:+44 1454 462330
[-- Attachment #2: asm-share-fix --]
[-- Type: text/plain, Size: 4016 bytes --]
After updating my sources today, I've seen compile/20010903-1.c
regressions for all four SH64 -m5-compat subtargets at optimization levels
-O3 -fomit-frame-pointer -funroll-loops and -O3 -fomit-frame-pointer
-funroll-all-loops -finline-functions.
(There are more regressions that I will address later.)
It seems the recent unroll changes have exposed a bug in the unsharing
code. We are not supposed to unshare memory operands in an asm from each
other. The instruction in question is:
(insn 42 38 48 1 0x401bf940 (parallel [
(set (mem/s:SI (reg/v/f:SI 167) [4 <variable>.a+0 S4 A32])
(asm_operands/v:SI ("") ("=m") 0 [
(reg/v/f:SI 167)
(mem/s:SI (reg/v/f:SI 167) [4 <variable>.a+0 S4 A32])
]
[
(asm_input:SI ("r"))
(asm_input:SI ("0"))
] ("/export/home/build/srcw/gcc/testsuite/gcc.c-torture/compile/20010903-1.c") 7))
(clobber (mem:BLK (scratch) [0 A8]))
]) -1 (nil)
(nil))
The destination and the second input are shared before unshare_all_rtl.
Since the memory address is variable, the MEM is unshared.
The following patch uses a special function to unshare assembler instructions
that can have multiple operands, which ensures that any sharing between its
operands is retained.
Tue Jul 23 16:46:47 2002 J"orn Rennecke <joern.rennecke@superh.com>
* emit-rtl.c (asm_copy_rtx_fix_shared): New function.
(unshare_all_rtl_1): Use it.
Index: emit-rtl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/emit-rtl.c,v
retrieving revision 1.286
diff -p -r1.286 emit-rtl.c
*** emit-rtl.c 22 Jul 2002 00:23:47 -0000 1.286
--- emit-rtl.c 23 Jul 2002 15:46:46 -0000
*************** static rtx find_line_note PARAMS ((rtx)
*** 173,178 ****
--- 173,179 ----
static rtx change_address_1 PARAMS ((rtx, enum machine_mode, rtx,
int));
static void unshare_all_rtl_1 PARAMS ((rtx));
+ static void asm_copy_rtx_if_shared PARAMS ((rtx, rtx));
static void unshare_all_decls PARAMS ((tree));
static void reset_used_decls PARAMS ((tree));
static void mark_label_nuses PARAMS ((rtx));
*************** unshare_all_rtl_1 (insn)
*** 2342,2351 ****
for (; insn; insn = NEXT_INSN (insn))
if (INSN_P (insn))
{
! PATTERN (insn) = copy_rtx_if_shared (PATTERN (insn));
REG_NOTES (insn) = copy_rtx_if_shared (REG_NOTES (insn));
LOG_LINKS (insn) = copy_rtx_if_shared (LOG_LINKS (insn));
}
}
/* Go through all virtual stack slots of a function and copy any
--- 2343,2393 ----
for (; insn; insn = NEXT_INSN (insn))
if (INSN_P (insn))
{
! rtx pat = PATTERN (insn);
!
! if (asm_noperands (pat) >= 0)
! asm_copy_rtx_if_shared (pat, pat);
! else
! PATTERN (insn) = copy_rtx_if_shared (pat);
REG_NOTES (insn) = copy_rtx_if_shared (REG_NOTES (insn));
LOG_LINKS (insn) = copy_rtx_if_shared (LOG_LINKS (insn));
}
+ }
+
+ /* Unshare X, which is part of an asm instruction PAT, but if X
+ might be an operand and it appears in other places of this same
+ pattern, preserve that sharing, by doing identical replacements there. */
+ static void
+ asm_copy_rtx_if_shared (pat, x)
+ rtx pat, x;
+ {
+ rtvec vec;
+ int i;
+
+ switch (GET_CODE (x))
+ {
+ case SET:
+ asm_copy_rtx_if_shared (pat, SET_DEST (x));
+ asm_copy_rtx_if_shared (pat, SET_SRC (x));
+ return;
+ case MEM:
+ default:
+ {
+ rtx y = copy_rtx_if_shared (x);
+
+ if (x != y)
+ replace_rtx (pat, x, y);
+ }
+ return;
+ case PARALLEL:
+ vec = XVEC (x, 0);
+ break;
+ case ASM_OPERANDS:
+ vec = ASM_OPERANDS_INPUT_VEC (x);
+ break;
+ }
+ for (i = GET_NUM_ELEM (vec) - 1; i >= 0; i--)
+ asm_copy_rtx_if_shared (pat, RTVEC_ELT (vec, i));
}
/* Go through all virtual stack slots of a function and copy any
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFA: Fix for compile/20010903-1.c regressions
2002-07-23 9:24 RFA: Fix for compile/20010903-1.c regressions Joern Rennecke
@ 2002-07-29 14:51 ` Richard Henderson
2002-07-30 6:47 ` Joern Rennecke
2002-08-20 8:55 ` Joern Rennecke
0 siblings, 2 replies; 6+ messages in thread
From: Richard Henderson @ 2002-07-29 14:51 UTC (permalink / raw)
To: Joern Rennecke; +Cc: gcc-patches, aj
On Tue, Jul 23, 2002 at 05:03:15PM +0100, Joern Rennecke wrote:
> After updating my sources today, I've seen compile/20010903-1.c
> regressions for all four SH64 -m5-compat subtargets at optimization levels
> -O3 -fomit-frame-pointer -funroll-loops and -O3 -fomit-frame-pointer
> -funroll-all-loops -finline-functions.
Yes, this happens on x86 as well.
> It seems the recent unroll changes have exposed a bug in the unsharing
> code. We are not supposed to unshare memory operands in an asm from each
> other.
I don't think it's the unsharing code. I assume you're getting the
same "inconsistent operand constraints in an `asm'" message? Looking
at the .loop rtl dump, the problem is that the memory matching contraint
operand is really different. This is not an unsharing problem but
rather an unrolling problem.
I get correct results if I go ahead and kill the rest of the DEST_ADDR
code adjacent to the bits I deleted last week. With this, the code
coming out of .loop looks pretty nasty, but .life and .combine clean
things up significantly.
Assuming test results come out ok, I guess I'm going to go ahead with
this. Andreas, can you see if this negates the Spec improvement we
got the other day?
r~
* unroll.c (verify_addresses): Remove.
(find_splittable_givs): Never split DEST_ADDR givs.
Index: unroll.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/unroll.c,v
retrieving revision 1.171
diff -c -p -d -r1.171 unroll.c
*** unroll.c 22 Jul 2002 00:29:56 -0000 1.171
--- unroll.c 29 Jul 2002 21:47:17 -0000
*************** static int find_splittable_givs PARAMS (
*** 207,213 ****
rtx, int));
static int reg_dead_after_loop PARAMS ((const struct loop *, rtx));
static rtx fold_rtx_mult_add PARAMS ((rtx, rtx, rtx, enum machine_mode));
- static int verify_addresses PARAMS ((struct induction *, rtx, int));
static rtx remap_split_bivs PARAMS ((struct loop *, rtx));
static rtx find_common_reg_term PARAMS ((rtx, rtx));
static rtx subtract_reg_term PARAMS ((rtx, rtx));
--- 207,212 ----
*************** find_splittable_regs (loop, unroll_type,
*** 2607,2641 ****
return result;
}
- /* Return 1 if the first and last unrolled copy of the address giv V is valid
- for the instruction that is using it. Do not make any changes to that
- instruction. */
-
- static int
- verify_addresses (v, giv_inc, unroll_number)
- struct induction *v;
- rtx giv_inc;
- int unroll_number;
- {
- int ret = 1;
- rtx orig_addr = *v->location;
- rtx last_addr = plus_constant (v->dest_reg,
- INTVAL (giv_inc) * (unroll_number - 1));
-
- /* First check to see if either address would fail. Handle the fact
- that we have may have a match_dup. */
- if (! validate_replace_rtx (*v->location, v->dest_reg, v->insn)
- || ! validate_replace_rtx (*v->location, last_addr, v->insn))
- ret = 0;
-
- /* Now put things back the way they were before. This should always
- succeed. */
- if (! validate_replace_rtx (*v->location, orig_addr, v->insn))
- abort ();
-
- return ret;
- }
-
/* For every giv based on the biv BL, check to determine whether it is
splittable. This is a subroutine to find_splittable_regs ().
--- 2606,2611 ----
*************** find_splittable_givs (loop, bl, unroll_t
*** 2647,2653 ****
struct iv_class *bl;
enum unroll_types unroll_type;
rtx increment;
! int unroll_number;
{
struct loop_ivs *ivs = LOOP_IVS (loop);
struct induction *v, *v2;
--- 2617,2623 ----
struct iv_class *bl;
enum unroll_types unroll_type;
rtx increment;
! int unroll_number ATTRIBUTE_UNUSED;
{
struct loop_ivs *ivs = LOOP_IVS (loop);
struct induction *v, *v2;
*************** find_splittable_givs (loop, bl, unroll_t
*** 2818,2924 ****
splittable_regs[REGNO (v->new_reg)] = value;
}
else
! {
! /* Splitting address givs is useful since it will often allow us
! to eliminate some increment insns for the base giv as
! unnecessary. */
!
! /* If the addr giv is combined with a dest_reg giv, then all
! references to that dest reg will be remapped, which is NOT
! what we want for split addr regs. We always create a new
! register for the split addr giv, just to be safe. */
!
! /* If we have multiple identical address givs within a
! single instruction, then use a single pseudo reg for
! both. This is necessary in case one is a match_dup
! of the other. */
!
! v->const_adjust = 0;
!
! if (v->same_insn)
! {
! v->dest_reg = v->same_insn->dest_reg;
! if (loop_dump_stream)
! fprintf (loop_dump_stream,
! "Sharing address givs in insn %d\n",
! INSN_UID (v->insn));
! }
! /* If multiple address GIVs have been combined with the
! same dest_reg GIV, do not create a new register for
! each. */
! else if (unroll_type != UNROLL_COMPLETELY
! && v->giv_type == DEST_ADDR
! && v->same && v->same->giv_type == DEST_ADDR
! && v->same->unrolled
! /* combine_givs_p may return true for some cases
! where the add and mult values are not equal.
! To share a register here, the values must be
! equal. */
! && rtx_equal_p (v->same->mult_val, v->mult_val)
! && rtx_equal_p (v->same->add_val, v->add_val)
! /* If the memory references have different modes,
! then the address may not be valid and we must
! not share registers. */
! && verify_addresses (v, giv_inc, unroll_number))
! {
! v->dest_reg = v->same->dest_reg;
! v->shared = 1;
! }
! else if (unroll_type == UNROLL_COMPLETELY)
! {
! v->dest_reg = value;
!
! /* Check the resulting address for validity, and fail
! if the resulting address would be invalid. */
! if (! verify_addresses (v, giv_inc, unroll_number))
! {
! for (v2 = v->next_iv; v2; v2 = v2->next_iv)
! if (v2->same_insn == v)
! v2->same_insn = 0;
!
! if (loop_dump_stream)
! fprintf (loop_dump_stream,
! "Invalid address for giv at insn %d\n",
! INSN_UID (v->insn));
! continue;
! }
! }
! else
! continue;
!
! /* Store the value of dest_reg into the insn. This sharing
! will not be a problem as this insn will always be copied
! later. */
!
! *v->location = v->dest_reg;
!
! /* If this address giv is combined with a dest reg giv, then
! save the base giv's induction pointer so that we will be
! able to handle this address giv properly. The base giv
! itself does not have to be splittable. */
!
! if (v->same && v->same->giv_type == DEST_REG)
! addr_combined_regs[REGNO (v->same->new_reg)] = v->same;
!
! if (GET_CODE (v->new_reg) == REG)
! {
! /* This giv maybe hasn't been combined with any others.
! Make sure that it's giv is marked as splittable here. */
!
! splittable_regs[REGNO (v->new_reg)] = value;
!
! /* Make it appear to depend upon itself, so that the
! giv will be properly split in the main loop above. */
! if (! v->same)
! {
! v->same = v;
! addr_combined_regs[REGNO (v->new_reg)] = v;
! }
! }
!
! if (loop_dump_stream)
! fprintf (loop_dump_stream, "DEST_ADDR giv being split.\n");
! }
}
else
{
--- 2788,2794 ----
splittable_regs[REGNO (v->new_reg)] = value;
}
else
! continue;
}
else
{
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFA: Fix for compile/20010903-1.c regressions
2002-07-29 14:51 ` Richard Henderson
@ 2002-07-30 6:47 ` Joern Rennecke
2002-07-30 9:43 ` Richard Henderson
2002-08-20 8:55 ` Joern Rennecke
1 sibling, 1 reply; 6+ messages in thread
From: Joern Rennecke @ 2002-07-30 6:47 UTC (permalink / raw)
To: Richard Henderson; +Cc: gcc-patches, aj
Richard Henderson wrote:
>
> On Tue, Jul 23, 2002 at 05:03:15PM +0100, Joern Rennecke wrote:
> > After updating my sources today, I've seen compile/20010903-1.c
> > regressions for all four SH64 -m5-compat subtargets at optimization levels
> > -O3 -fomit-frame-pointer -funroll-loops and -O3 -fomit-frame-pointer
> > -funroll-all-loops -finline-functions.
>
> Yes, this happens on x86 as well.
>
> > It seems the recent unroll changes have exposed a bug in the unsharing
> > code. We are not supposed to unshare memory operands in an asm from each
> > other.
>
> I don't think it's the unsharing code. I assume you're getting the
> same "inconsistent operand constraints in an `asm'" message? Looking
> at the .loop rtl dump, the problem is that the memory matching contraint
> operand is really different. This is not an unsharing problem but
> rather an unrolling problem.
If the code starts out shared, it is kept shared and hence, identical.
--
--------------------------
SuperH
2430 Aztec West / Almondsbury / BRISTOL / BS32 4AQ
T:+44 1454 462330
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFA: Fix for compile/20010903-1.c regressions
2002-07-30 6:47 ` Joern Rennecke
@ 2002-07-30 9:43 ` Richard Henderson
0 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2002-07-30 9:43 UTC (permalink / raw)
To: Joern Rennecke; +Cc: gcc-patches, aj
On Tue, Jul 30, 2002 at 11:17:37AM +0100, Joern Rennecke wrote:
> If the code starts out shared, it is kept shared and hence, identical.
Well, yes and no. There is no constraint that the destination
memory and the source memory of an asm must be shared. Yet
that is how you got things to work.
I'd been thinking a simpler solution than writing a new function
to properly unshare asms would be to use copy_insn (i.e. create
an entirely new rtl hunk), which does know about the sharing
requirement of asms and would preserve them properly. Except
that that didn't solve the problem.
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFA: Fix for compile/20010903-1.c regressions
2002-07-29 14:51 ` Richard Henderson
2002-07-30 6:47 ` Joern Rennecke
@ 2002-08-20 8:55 ` Joern Rennecke
2002-08-20 10:09 ` Richard Henderson
1 sibling, 1 reply; 6+ messages in thread
From: Joern Rennecke @ 2002-08-20 8:55 UTC (permalink / raw)
To: Richard Henderson; +Cc: gcc-patches, aj
Richard Henderson wrote:
>
> On Tue, Jul 23, 2002 at 05:03:15PM +0100, Joern Rennecke wrote:
> > After updating my sources today, I've seen compile/20010903-1.c
> > regressions for all four SH64 -m5-compat subtargets at optimization levels
> > -O3 -fomit-frame-pointer -funroll-loops and -O3 -fomit-frame-pointer
> > -funroll-all-loops -finline-functions.
>
> Yes, this happens on x86 as well.
>
> > It seems the recent unroll changes have exposed a bug in the unsharing
> > code. We are not supposed to unshare memory operands in an asm from each
> > other.
>
> I don't think it's the unsharing code. I assume you're getting the
> same "inconsistent operand constraints in an `asm'" message? Looking
> at the .loop rtl dump, the problem is that the memory matching contraint
> operand is really different. This is not an unsharing problem but
> rather an unrolling problem.
>
> I get correct results if I go ahead and kill the rest of the DEST_ADDR
> code adjacent to the bits I deleted last week. With this, the code
> coming out of .loop looks pretty nasty, but .life and .combine clean
> things up significantly.
However, they can't remove excess strength-reduced givs.
I think we should use flow information to determine if a biv
is dead outside of a loop; it is all too common that the same
variable is used over and over again as a loop counter, and
then a lot of loop optimizations are hamstringed.
-----
2430 Aztec West / Almondsbury / BRISTOL / BS32 4AQ
T:+44 1454 462330
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFA: Fix for compile/20010903-1.c regressions
2002-08-20 8:55 ` Joern Rennecke
@ 2002-08-20 10:09 ` Richard Henderson
0 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2002-08-20 10:09 UTC (permalink / raw)
To: Joern Rennecke; +Cc: gcc-patches, aj
On Tue, Aug 20, 2002 at 04:53:27PM +0100, Joern Rennecke wrote:
> I think we should use flow information to determine if a biv
> is dead outside of a loop ...
Sure. I'll be happy to look at a patch for that. It'll prolly
have to wait until 3.4 to go in, unfortunately...
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2002-08-20 17:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-07-23 9:24 RFA: Fix for compile/20010903-1.c regressions Joern Rennecke
2002-07-29 14:51 ` Richard Henderson
2002-07-30 6:47 ` Joern Rennecke
2002-07-30 9:43 ` Richard Henderson
2002-08-20 8:55 ` Joern Rennecke
2002-08-20 10:09 ` Richard Henderson
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).