This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Re: [new regalloc]: Miscompile for s390x
- From: Michael Matz <matz at suse dot de>
- To: Hartmut Penner <HPENNER at de dot ibm dot com>
- Cc: <gcc at gcc dot gnu dot org>, <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 17 Jun 2002 16:02:27 +0200 (CEST)
- Subject: Re: [new regalloc]: Miscompile for s390x
Hi Hartmut,
On Mon, 17 Jun 2002, Hartmut Penner wrote:
> while running regtest for S390x on the regalloc branch, found that
> c-format.c is miscompiled.
> Boiled it down to following:
Many thanks for the very helpful analyze. The below diff (which I checked
in) should help here.
> L280: ; [3 uses] uid=(4151)
> 5412 p78 <= 0 <- Look like wrong rematerializations
Yes, if the first found def was rematerializable, but just no
side-effect-free insn could be found to regenerate it, then this info was
lost, and the remaining defs (those sets to zero) were regarded as the
only defs.
Ciao,
Michael.
--
* ra.c (detect_remat_webs): Fail if the first found pattern
is not rematerializable.
Index: ra.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/Attic/ra.c,v
retrieving revision 1.1.2.56
diff -u -p -r1.1.2.56 ra.c
--- ra.c 13 Jun 2002 08:41:26 -0000 1.1.2.56
+++ ra.c 17 Jun 2002 13:53:08 -0000
@@ -3750,30 +3750,30 @@ detect_remat_webs (void)
if (pat)
continue;
/* For now we allow only constant sources. */
- if (CONSTANT_P (src)
- /* If the whole thing is stable already, it is a source for
- remat, no matter how complicated (probably all needed
- resources for it are live everywhere, and don't take
- additional register resources). */
- /* XXX Currently we can't use patterns which contain
- pseudos, _even_ if they are stable.
- The code simply isn't prepared for that. All those
- operands can't be spilled (or the dependent remat webs are
- not remat anymore), so they would be oldwebs in the next
- iteration. But currently oldwebs can't have their references
- changed. The incremental machinery barfs on that. */
- || (!rtx_unstable_p (src) && !contains_pseudo (src))
- /* Additionally also memrefs to stack-slots are usefull, when
- we created them ourself. They might not have set their
- unchanging flag set, but nevertheless they are stable
- across the livetime in question. */
- || (GET_CODE (src) == MEM
- && INSN_UID (insn) >= orig_max_uid
- && memref_is_stack_slot (src)))
- {
- if (want_to_remat (src))
- pat = src;
- }
+ if ((CONSTANT_P (src)
+ /* If the whole thing is stable already, it is a source for
+ remat, no matter how complicated (probably all needed
+ resources for it are live everywhere, and don't take
+ additional register resources). */
+ /* XXX Currently we can't use patterns which contain
+ pseudos, _even_ if they are stable. The code simply isn't
+ prepared for that. All those operands can't be spilled (or
+ the dependent remat webs are not remat anymore), so they
+ would be oldwebs in the next iteration. But currently
+ oldwebs can't have their references changed. The
+ incremental machinery barfs on that. */
+ || (!rtx_unstable_p (src) && !contains_pseudo (src))
+ /* Additionally also memrefs to stack-slots are usefull, when
+ we created them ourself. They might not have set their
+ unchanging flag set, but nevertheless they are stable across
+ the livetime in question. */
+ || (GET_CODE (src) == MEM
+ && INSN_UID (insn) >= orig_max_uid
+ && memref_is_stack_slot (src)))
+ /* And we must be able to construct an insn without
+ side-effects to actually load that value into a reg. */
+ && want_to_remat (src))
+ pat = src;
else
break;
}