This is the mail archive of the
mailing list for the GCC project.
Re: [RFC] PR/27390, latent complex-mode bug in regstack
- From: Roger Sayle <roger at eyesopen dot com>
- To: Paolo Bonzini <paolo dot bonzini at lu dot unisi dot ch>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Richard Guenther <rguenther at suse dot de>
- Date: Mon, 5 Jun 2006 18:08:11 -0600 (MDT)
- Subject: Re: [RFC] PR/27390, latent complex-mode bug in regstack
On Mon, 5 Jun 2006, Paolo Bonzini wrote:
> This patch fixes two problems in regstack involving complex modes.
Sorry again for the delay in commenting on PR target/27390, it's
another case of someone Cc'ing firstname.lastname@example.org in bugzilla. :-(
I believe something like your first?/second? patch, that only affects
subst_stack_regs_pat is the correct fix. You describe them as patch
one and patch two in your original message, but they appear in reversed
order in the ENOPATCH follow-up. :-)
The issue (which it sounds like you've already figured out), is that
move_nan_for_stack_reg must be called in the same order as the
instructions are processed/encountered in the insn stream in order to
keep the "regstack" state (which is updated in move_nan_for_stack_reg)
The problems you described in bugzilla, are from your original attempt
calling "move_nan_for_stack_reg" on insn, then inserting a new insn
immediately before it, and calling "move_nan_for_stack_reg" on that:
which confused the "regstack" state as to which registers were where
on the stack after this sequence.
Notice, that this resulted in the invalid code:
when what the compiler intended to generate was
Hence your revised patch that inserts the new insn first, calls
move_nan_for_stack_reg on it, and then calls move_nan_for_stack
on the current/final insn is the correct strategy.
Ideally, we can probably do even better by converting this into
avoiding two references to the constant pool, by first loading
a NaN, and then duplicating the top of stack. Not that uninitialized
complex values are a common occurence :-). By peaking into the
inner workings for "move_nan_for_stack_reg" you can probably avoid
creating the CLOBBER pattern you currently use, and call directly
to "move_for_stack_reg", first for the NaN then for the copy/dup.
This avoids creating dummy RTL only to immediately overwrite/leak it.
Does all this make sense? (and rationalizes the strange behaviours
you've been seeing). Let me know if your happy to investigate the above
refinements or you'd prefer to commit something similar to what you have
(to resolve the regressions) and leave it to someone else to tidy up
Many thanks for investigating this. It's now clear that this was a
pre-existing latent bug that your reload preference changes had