* Re: [patch] modify crt0.S for 64-bit address targets
@ 2005-04-06 18:51 cgd
2005-04-06 19:01 ` Eric Christopher
0 siblings, 1 reply; 10+ messages in thread
From: cgd @ 2005-04-06 18:51 UTC (permalink / raw)
To: cgd, echristo; +Cc: binutils, newlib
> Assuredly this is the case. I think the problem might be from using the
> ori instead of addi for the address.
Perhaps. For a 32-bit address, I'd expect the address generation to be
simply lui/addiu, or similar. (in fact, for this, you only need the
lui.)
The only time i'd expect gas would be bothering with shifts at all
is for 64-bit addresses. (but that's intuition speaking, rather than
code examination.)
cgd
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] modify crt0.S for 64-bit address targets 2005-04-06 18:51 [patch] modify crt0.S for 64-bit address targets cgd @ 2005-04-06 19:01 ` Eric Christopher 0 siblings, 0 replies; 10+ messages in thread From: Eric Christopher @ 2005-04-06 19:01 UTC (permalink / raw) To: cgd; +Cc: binutils, newlib On Wed, 2005-04-06 at 11:51 -0700, cgd@broadcom.com wrote: > > Assuredly this is the case. I think the problem might be from using the > > ori instead of addi for the address. > > Perhaps. For a 32-bit address, I'd expect the address generation to be > simply lui/addiu, or similar. (in fact, for this, you only need the > lui.) > Agreed. > The only time i'd expect gas would be bothering with shifts at all > is for 64-bit addresses. (but that's intuition speaking, rather than > code examination.) Technically this is a 64-bit address target (ABI_EABI and a processor with 64-bit registers). For this particular address I think all you need is the addi and the shift, but... -eric ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <1112768205.5493.2.camel@localhost.localdomain>]
[parent not found: <mailpost.1112768909.17118@news-sj1-1>]
* Re: [patch] modify crt0.S for 64-bit address targets [not found] ` <mailpost.1112768909.17118@news-sj1-1> @ 2005-04-06 6:49 ` cgd 2005-04-06 11:05 ` Maciej W. Rozycki 2005-04-06 18:37 ` Eric Christopher 0 siblings, 2 replies; 10+ messages in thread From: cgd @ 2005-04-06 6:49 UTC (permalink / raw) To: echristo; +Cc: newlib, binutils (added cc: to binutils list.) Eric, My take on this is that the problem is **not** that addu/subu are incorrect, but rather that the address being loaded is not what's intended! The intended address (K0BASE) is 0xffffffff80000000, and based on your comments (and the detection of the UNPREDICTABLE condition), I believe that 0x0000000080000000 is being loaded instead. The address is a sign-extended 32-bit value, the addu/subu should be OK. If the simulator's actually doing the right thing, and the code really is what your description below indicates, the changed code shouldn't work any better. (I forget, i have some recollection that the current sim might always truncate addresses to 32-bits. but it's been a while since i looked at that. *sigh*) I'm probably confused/mistaken but... i thought 'la' was supposed to generate a 32-bit address (and dla generate a 64-bit address)? Or maybe they always generate the address of the ABI in use? I forget... so much change. chris At Wed, 6 Apr 2005 06:28:30 +0000 (UTC), "Eric Christopher" wrote: > So, in testing mipsisa64-elf lately we were getting a number of > UNPREDICTABLE results because of this: > > la t1,K0BASE > addu t0,t0,t1 > subu t0,t0,64 > > Which would assemble to this: > > ori > dsll > > addu > subu > > The problem is that ABI_EABI for 64-bit targets assumes 64-bit addresses > and the doubleword shift meant that register t0 would get something that > isn't a word value, causing unpredictable results depending on > processor. The mips simulator flags these. Now, shifting this to using > 64-bit instructions for 64-bit addresses seems to be the best method for > this. I only changed the ones that the simulator would catch thinking it > best that we only change the ones we need to. I'm perfectly happy to > switch it to using 64-bit instructions for everything for 64-bit, but I > think that's probably a bit much. > > Tested on mipsisa64-elf and mips64-elf. > > OK? > > 2005-04-05 Eric Christopher <echristo@redhat.com> > > * mips/crt0.S: Add macros for 64-bit add/sub. > (zerobss): Use when dealing with addresses. > > Index: crt0.S > =================================================================== > RCS file: /cvs/src/src/libgloss/mips/crt0.S,v > retrieving revision 1.9 > diff -u -p -w -r1.9 crt0.S > --- crt0.S 26 May 2003 20:22:16 -0000 1.9 > +++ crt0.S 6 Apr 2005 02:02:55 -0000 > @@ -41,6 +41,16 @@ > # define LA(t,x) la t,x > #endif /* __mips_embedded_pic */ > > +#if !defined(__mips64) > + /* This machine does not support 64-bit operations. */ > + #define ADDU addu > + #define SUBU subu > +#else > + /* This machine supports 64-bit operations. */ > + #define ADDU daddu > + #define SUBU dsubu > +#endif > + > .comm __memsize, 12 > .comm __lstack, STARTUP_STACK_SIZE > > @@ -156,11 +166,11 @@ zerobss: > la a0, __memsize > lw t0,0(a0) # last address of memory available > la t1,K0BASE # cached kernel memory > - addu t0,t0,t1 # get the end of memory address > + ADDU t0,t0,t1 # get the end of memory address > /* Allocate 32 bytes for the register parameters. Allocate 16 > bytes for a null argv and envp. Round the result up to 64 > bytes to preserve alignment. */ > - subu t0,t0,64 > + SUBU t0,t0,64 > 4: > move sp,t0 # set stack pointer > .end zerobss ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] modify crt0.S for 64-bit address targets 2005-04-06 6:49 ` cgd @ 2005-04-06 11:05 ` Maciej W. Rozycki 2005-04-06 11:22 ` Thiemo Seufer 2005-04-06 18:37 ` Eric Christopher 1 sibling, 1 reply; 10+ messages in thread From: Maciej W. Rozycki @ 2005-04-06 11:05 UTC (permalink / raw) To: cgd; +Cc: echristo, newlib, binutils On Wed, 5 Apr 2005 cgd@broadcom.com wrote: > I'm probably confused/mistaken but... i thought 'la' was supposed to > generate a 32-bit address (and dla generate a 64-bit address)? Or > maybe they always generate the address of the ABI in use? I > forget... so much change. AFAIK, the only reason for both "dla" and "la" to exist is history and compatibility with existing code -- there is no need to encode the address size in the mnemonic as its already implied by the ABI in use. Therefore, as far as gas is concerned, both expand to the same code -- it's just they generate different warning messages and for different conditions: "dla" complains about being used for ABIs that have 32-bit registers and "la" does that for ones that have 64-bit addresses. Maciej ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] modify crt0.S for 64-bit address targets 2005-04-06 11:05 ` Maciej W. Rozycki @ 2005-04-06 11:22 ` Thiemo Seufer 2005-04-06 14:40 ` Maciej W. Rozycki 0 siblings, 1 reply; 10+ messages in thread From: Thiemo Seufer @ 2005-04-06 11:22 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: cgd, echristo, newlib, binutils Maciej W. Rozycki wrote: > On Wed, 5 Apr 2005 cgd@broadcom.com wrote: > > > I'm probably confused/mistaken but... i thought 'la' was supposed to > > generate a 32-bit address (and dla generate a 64-bit address)? Or > > maybe they always generate the address of the ABI in use? I > > forget... so much change. > > AFAIK, the only reason for both "dla" and "la" to exist is history and > compatibility with existing code -- there is no need to encode the address > size in the mnemonic as its already implied by the ABI in use. This is incorrect for the (historic) no-ABI mode, as well as for e.g. a .set mips3 ... .set mips0 sequence. la should load a sign-extended 32bit value in that case. Thiemo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] modify crt0.S for 64-bit address targets 2005-04-06 11:22 ` Thiemo Seufer @ 2005-04-06 14:40 ` Maciej W. Rozycki 0 siblings, 0 replies; 10+ messages in thread From: Maciej W. Rozycki @ 2005-04-06 14:40 UTC (permalink / raw) To: Thiemo Seufer; +Cc: cgd, echristo, newlib, binutils On Wed, 6 Apr 2005, Thiemo Seufer wrote: > > AFAIK, the only reason for both "dla" and "la" to exist is history and > > compatibility with existing code -- there is no need to encode the address > > size in the mnemonic as its already implied by the ABI in use. > > This is incorrect for the (historic) no-ABI mode, as well as for e.g. a What's a no-ABI mode? There's always an ABI present, whether explicit or not. For the purpose of addressing, using 32-bit ELF implies 32-bit addresses, while using 64-bit ELF means 64-bit addresses. > .set mips3 ... .set mips0 sequence. la should load a sign-extended 32bit > value in that case. It will as these don't affect the address size -- la uses the address size that reflects the ABI selected, unless you restrict it with something like ".set mips1". But using ".set mips1", ".set mips2" and ".set mips32" should probably be forbidden for ABIs that require a 64-bit ISA, just like option sets like "-mabi=64 -march=mips1". Maciej ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] modify crt0.S for 64-bit address targets 2005-04-06 6:49 ` cgd 2005-04-06 11:05 ` Maciej W. Rozycki @ 2005-04-06 18:37 ` Eric Christopher 2005-04-06 19:31 ` Paul Koning 1 sibling, 1 reply; 10+ messages in thread From: Eric Christopher @ 2005-04-06 18:37 UTC (permalink / raw) To: cgd; +Cc: newlib, binutils On Tue, 2005-04-05 at 23:46 -0700, cgd@broadcom.com wrote: > (added cc: to binutils list.) > > Eric, > > My take on this is that the problem is **not** that addu/subu are > incorrect, but rather that the address being loaded is not what's > intended! > > The intended address (K0BASE) is 0xffffffff80000000, and based on your > comments (and the detection of the UNPREDICTABLE condition), I believe > that 0x0000000080000000 is being loaded instead. > Assuredly this is the case. I think the problem might be from using the ori instead of addi for the address. ori zero extends the constant, addi sign extends. IIRC all addresses should be sign extended and as such we should be using addi here yes? I seem to recall that we changed this a while back because the ori was some small amount faster. Maciej? Thiemo? This ringing any bells? > The address is a sign-extended 32-bit value, the addu/subu should be > OK. You are correct. -eric ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] modify crt0.S for 64-bit address targets 2005-04-06 18:37 ` Eric Christopher @ 2005-04-06 19:31 ` Paul Koning 2005-04-06 19:35 ` Ian Lance Taylor 2005-04-06 21:35 ` Eric Christopher 0 siblings, 2 replies; 10+ messages in thread From: Paul Koning @ 2005-04-06 19:31 UTC (permalink / raw) To: echristo; +Cc: cgd, newlib, binutils >>>>> "Eric" == Eric Christopher <echristo@redhat.com> writes: Eric> On Tue, 2005-04-05 at 23:46 -0700, cgd@broadcom.com wrote: >> (added cc: to binutils list.) >> >> Eric, >> >> My take on this is that the problem is **not** that addu/subu are >> incorrect, but rather that the address being loaded is not what's >> intended! >> >> The intended address (K0BASE) is 0xffffffff80000000, and based on >> your comments (and the detection of the UNPREDICTABLE condition), >> I believe that 0x0000000080000000 is being loaded instead. >> Eric> Assuredly this is the case. I think the problem might be from Eric> using the ori instead of addi for the address. ori zero extends Eric> the constant, addi sign extends. IIRC all addresses should be Eric> sign extended and as such we should be using addi here yes? I Eric> seem to recall that we changed this a while back because the Eric> ori was some small amount faster. Faster? On a RISC machine? Seems odd. Anyway, ori is fine. The sequence for LA is LUI then ORI, and LUI loads a sign-extended 32 bit value with zeroes in the low order. So ORI is the easy way to fill in the low order. The LUI is what ensures that the value is formatted correctly in 64 bit registers. ADDI or ADDIU both sign-extend the 16-bit argument, so doing an LA with one of those, while possible, is more trouble because the LUI argument has to be adjusted up by 1 if the low half of the value to load is > 7fff. With ORI, the argument is zero-extended so it the low bits aren't a consideration when picking the LUI argument paul ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] modify crt0.S for 64-bit address targets 2005-04-06 19:31 ` Paul Koning @ 2005-04-06 19:35 ` Ian Lance Taylor 2005-04-06 21:35 ` Eric Christopher 1 sibling, 0 replies; 10+ messages in thread From: Ian Lance Taylor @ 2005-04-06 19:35 UTC (permalink / raw) To: Paul Koning; +Cc: echristo, cgd, newlib, binutils Paul Koning <pkoning@equallogic.com> writes: > Eric> Assuredly this is the case. I think the problem might be from > Eric> using the ori instead of addi for the address. ori zero extends > Eric> the constant, addi sign extends. IIRC all addresses should be > Eric> sign extended and as such we should be using addi here yes? I > Eric> seem to recall that we changed this a while back because the > Eric> ori was some small amount faster. > > Faster? On a RISC machine? Seems odd. I think Eric is reversed, actually. There was a MIPS processor which had two add units but only one logical unit. Therefore, on average, on that processor, add was faster. That fact led to this change: Tue Jul 11 11:49:49 1995 Ian Lance Taylor <ian@cygnus.com> * mips-opc.c (mips_opcodes): For the move pseudo-op, prefer daddu if ISA 3 and addu otherwise, replacing or, since some MIPS chips have multiple add units but only a single logical unit. Ian ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] modify crt0.S for 64-bit address targets 2005-04-06 19:31 ` Paul Koning 2005-04-06 19:35 ` Ian Lance Taylor @ 2005-04-06 21:35 ` Eric Christopher 1 sibling, 0 replies; 10+ messages in thread From: Eric Christopher @ 2005-04-06 21:35 UTC (permalink / raw) To: Paul Koning; +Cc: cgd, newlib, binutils > Anyway, ori is fine. The sequence for LA is LUI then ORI, and LUI > loads a sign-extended 32 bit value with zeroes in the low order. So > ORI is the easy way to fill in the low order. The LUI is what ensures > that the value is formatted correctly in 64 bit registers. > I think "the sequence" is probably a bit much. Right now we're not doing LUI, and merely going with load register because the code sequence looks like this: la t1,K0BASE which has K0BASE defined to 0x80000000, so all we're really doing is putting a value into a register and then doing work off of that. Really, I suppose we should just be putting the immediate in the register, instead of treating it as an address, e.g. li t1,K0BASE which does cause the unpredictability problems to go away. The code that's loading the value is load_register and it's the last set of code in the function if you want to look. > ADDI or ADDIU both sign-extend the 16-bit argument, so doing an LA > with one of those, while possible, is more trouble because the LUI > argument has to be adjusted up by 1 if the low half of the value to > load is > 7fff. With ORI, the argument is zero-extended so it the low > bits aren't a consideration when picking the LUI argument Fair enough, it was just a thought. I hate macros and so figuring out what we really mean when "blah" is annoying. -eric ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2005-04-06 21:35 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2005-04-06 18:51 [patch] modify crt0.S for 64-bit address targets cgd 2005-04-06 19:01 ` Eric Christopher [not found] <1112768205.5493.2.camel@localhost.localdomain> [not found] ` <mailpost.1112768909.17118@news-sj1-1> 2005-04-06 6:49 ` cgd 2005-04-06 11:05 ` Maciej W. Rozycki 2005-04-06 11:22 ` Thiemo Seufer 2005-04-06 14:40 ` Maciej W. Rozycki 2005-04-06 18:37 ` Eric Christopher 2005-04-06 19:31 ` Paul Koning 2005-04-06 19:35 ` Ian Lance Taylor 2005-04-06 21:35 ` Eric Christopher
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).