public inbox for cgen@sourceware.org
 help / color / mirror / Atom feed
* [RFC] cgen_parse_signed_integer() and 64bit bugs
@ 2006-08-12 17:20 Alexander Viro
  2006-08-14 20:24 ` Dave Brolley
  0 siblings, 1 reply; 2+ messages in thread
From: Alexander Viro @ 2006-08-12 17:20 UTC (permalink / raw)
  To: cgen

[apologies if it ends up double-posted]

Consider the following example (on FRV):
        setlos #0xffffffff,gr4
FRV is a 32bit target and instruction above takes a 16bit immediate
that gets sign-expanded.  So when we run native gas(1) or cross-build
on e.g. i386 we will get no complaints:
	1 0000 88FCFFFF              setlos #0xffffffff,gr4
Everything is fine - 16bit all-ones gets sign-expanded to 32bit all-ones.
Perhaps writing it as
	setlos #-1,gr4
would be cleaner, perhaps it would not, but in any case gas takes either
form without any complaints and produces expected results.

Unfortunately, as soon as you move to 64bit host, you'll get breakage.
Note that this is not a theoretical example - it's pulled straight from
arch/frv/lib/tlb-flush.S in Linux kernel.  Similar examples for other
architectures exist in a lot of places waiting for cross-toolchain to
be moved to another host.

Mechanism of this particular breakage is pretty simple:
  return cgen_parse_signed_integer (cd, strp, opindex, valuep);
in the very end of parse_uslo16() in cpu/frv.opc is sensitive to
word size of _host_.  That's documented behaviour, so in this
case the obvious answer would be "so truncate the value and sign-expand
it after the call".  As the matter of fact, _any_ 32bit target using
that puppy should be doing that.  Each case where such fixup is not
done means a portability bug.  And such bugs exist:
	* frv.opc never does fixups.  Each instance is a bug.
	* m32r.opc never does fixups.  Buggy.
	* m32c.opc never does fixups.  Buggy.
	* mt.opc has something that might be a fixup, but it's (a) broken
and (b) #if 0'd.  Buggy.
	* iq2000.opc does manual fixups.  Correct.
That's 4 out of 5.

Unfortunately, the situation is even messier.  We can do manual fixups,
all right.  But every time we describe something as h-sint we get a call
inserted into *-asm.c.  And for those we can't deal with that way.

Potential solutions:
	* deal with those suckers manually in frv-asm.c, etc.  Unacceptable,
since fixes will be lost the next time somebody reruns cgen.
	* add cgen_parse_signed_32bit() that would do fixups and h-s32 in
addition to h-sint.  Teach cgen to produce cgen_parse_signed_32bit() for
those.  Possible, but requires messing with cgen guts.
	* change the rules and make cgen_parse_signed_integer() to do
fixups.  Unfortunately, that means changes of behaviour for sh64-*.cpu.
All uses of h-sint in there are for 32 or fewer bits, so we are not
breaking any valid code, but currently we *do* fail for 0xffffffff as
argument and after that change we won't.  Plus there might be out-of-tree
users in similar situations.

Suggestions?

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [RFC] cgen_parse_signed_integer() and 64bit bugs
  2006-08-12 17:20 [RFC] cgen_parse_signed_integer() and 64bit bugs Alexander Viro
@ 2006-08-14 20:24 ` Dave Brolley
  0 siblings, 0 replies; 2+ messages in thread
From: Dave Brolley @ 2006-08-14 20:24 UTC (permalink / raw)
  To: Alexander Viro; +Cc: cgen

Alexander Viro wrote:

>	* change the rules and make cgen_parse_signed_integer() to do
>fixups. 
>  
>
I can't comment on the original intent, but this is how I've always 
expected this function to work.

Dave

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2006-08-14 20:24 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-08-12 17:20 [RFC] cgen_parse_signed_integer() and 64bit bugs Alexander Viro
2006-08-14 20:24 ` Dave Brolley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).