This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [Patch - hppa] add compile time check for immediate values of shift/deposit/extract
- From: "John David Anglin" <dave at hiauly1 dot hia dot nrc dot ca>
- To: deller at gmx dot de (Helge Deller)
- Cc: binutils at sourceware dot org, dave dot anglin at nrc dot ca, amodra at bigpond dot net dot au
- Date: Sun, 22 Feb 2009 14:40:26 -0500 (EST)
- Subject: Re: [Patch - hppa] add compile time check for immediate values of shift/deposit/extract
> The patch below adds compile time checks to gas which (where possible)
> will check the constant immediate values of the shift/deposit/extract
> functions and warn the user if he coded a dangerous combination.
This is a good idea.
> * gas/config/tc-hppa.c: Add check of immediate values.
This can be improved. Mention new define. The function, pa_ip, where
the change is made should be added after the file name.
> +/* Store immediate values of shift/deposit/extract functions. */
> +
> +#define INSERT_IMMEDIATE(VALUE) \
Suggest SAVE_IMMEDIATE would be a better name for macro.
> + { \
> + if (immediate_check) \
> + { if (sv1 == -1) sv1 = VALUE; \
> + else if (sv2 == -1) sv2 = VALUE; \
GNU style would place "if" on separate line from "{". I would rename
sv1 and sv2 to pos and len to correspond to the terminology used in the
arch. Hopefully, pos and len aren't already used.
> + if (immediate_check) {
Paren should be on next line and indented two spaces.
> + if (sv1 != -1 && sv2 != -1 && (sv2-1) > sv1)
The parens around "sv2-1" are unnecessary. There should be one space before
and after "-". Rewritten, this would look like:
if (pos != -1 && len != -1 && pos < len - 1)
This matches the undefined test in the arch.
Thanks,
Dave
--
J. David Anglin dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada (613) 990-0752 (FAX: 952-6602)