This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: [PATCH] ia64: spill_mask generation corrections
- From: James E Wilson <wilson at specifix dot com>
- To: Jan Beulich <JBeulich at novell dot com>
- Cc: binutils at sourceware dot org
- Date: Wed, 06 Jul 2005 13:36:30 -0700
- Subject: Re: [PATCH] ia64: spill_mask generation corrections
- References: <s2ca3420.045@lucius.provo.novell.com>
On Tue, 2005-07-05 at 06:18, Jan Beulich wrote:
> Besides dealing with the improper generation of spill_mask when
> .save.g,
> .save.gf, .save.f, or .save.b have more than one bit set in their
> operand(s),
> and besides adding another diagnostic for improper unwind directive
> usage,
> this patch also reduces the size of some of the tracking structures by
> combining a few fields that can never be used simultaneously into
> unions.
This looks OK, though I think you are trying to be too clever in a few
places.
For instance, in check_pending_save, you have
+ else
+ cur = (prev = cur)->next;
You are trying too hard to avoid using braces here.
In 5 different places, you have inside a loop
+ mask &= ~(mask & (~mask + 1));
+ if (!mask)
+ return ptr;
+ cur = alloc_record (fr_mem);
+ cur->r.record.p.frmask = mask;
+ prev->r.record.p.frmask ^= mask;
I understand why you wrote it that way, but I don't think you can safely
expect that everyone looking at this code in the next 20 years will
understand it. There should be comments explaining what it is doing.
First you clear off the low order bit of mask. Then you xor it into
prev frmask so that prev frmask is left with only the low order bit.
Then you repeat until no bits are left.
--
Jim Wilson, GNU Tools Support, http://www.specifix.com