This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PING] Re: SUBREGs and objects with holes


I think some general comments might be in order.

Firstly, I'm not very interested in any analysis that considers only e500 and x86. This routine is in generic code and should work for all ports. It should work even for ports that don't exist yet. The only way to be sure that a proposed solution is correct is to use logic to verify that the routine works. I do not think you can get there by saying "let's try to detect each possible case".

Secondly, the routine should work for all documented inputs. I don't see any comment saying "SImode is not a valid value for ymode", so it should either work for that, or such a comment should be added (with an assert to enforce it)---and I think you'll find adding such a comment+assert is not a good idea.

Thirdly, I think you're on the right track with the change to nregs_ymode, but my previous message pointed out two problems that need addressing in that change.

On 14/08/2006, at 1:32 PM, David Edelsohn wrote:

...
However, your patch can change FPR word units into GPR word units.

I think I don't understand what a 'fpr word unit' is. I understood an E500 didn't have FPRs, FP computation is done in GPRs. I vaguely see what you mean, but the vagueness is in the place where you are pointing out a bug, so...


Also, the dimensional analysis referred to the assert, not the final result.

Aaah. I did not see that from your mail.


However, the assert is, I believe, testing a case which wasn't analysed during the creation of the original routine. So it doesn't necessarily have to have any particular meaning.

The result is dimensionally consistent, except that nregs_xmode changed units.

I would be curious to see this analysis. Although clearly any inconsistency is the same as before, I am not sure that it is consistent. I think that might actually be part of the problem.


	My message actually contained two proposed patches and we are
trying to include your input in the decision about which to apply.

You can take my input as "neither of these patches should be applied as-is".


	One patch maintains consistency in the computation of nregs_xmode
and nregs_ymode consistent.  If one uses

nregs_xmode_unit_int * GET_MODE_NUNITS

instead of hard_regno_nregs, the other does as well.

I can't tell which patch(es) you are talking about here, nor which of the dozens of uses of hard_regno_nregs you refer to.


	The other patch restricts the alternate computation of nregs_xmode
more severely.

... is this the same as the 'other' you referred to in the previous sentence? ...


Currently your algorithm tests

nregs_xmode_unit != nregs_xmode_unit_int

It appears that your patch and reference to "holes" was concerned about
xmode and int_mode_for_mode (xmode) not being evenly divisible.

No, my patch really was concerned with the case where a value takes a different number of registers in its mode than it would have taken in an integer mode. Divisibility seems like an arbitrary way of 'detecting x86', see my first comment at the top.


Ideally, this test would not be necessary, and the code could use this code path all the time. I think that would make the code cleaner. Obviously it would require careful thought.

Yet another possibility is to restrict the computation to the case
where one operand is INTEGRAL_MODE_P and the other is FLOAT_MODE_P. This
also would capture the DCmode and SImode case and capture your comments
above.

I did not intend the comment to be an exhaustive list of the cases that can go wrong, merely a single example to show the general problem.

Attachment: smime.p7s
Description: S/MIME cryptographic signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]