public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Richard Sandiford <rdsandiford@googlemail.com>
To: "Maciej W. Rozycki" <macro@codesourcery.com>
Cc: Ilie Garbacea <ilie@mips.com>,
	 Joseph Myers <joseph@codesourcery.com>,
	 binutils@sourceware.org,  Chao-ying Fu <fu@mips.com>,
	 Rich Fuhler <rich@mips.com>,  David Lau <davidlau@mips.com>,
	 Kevin Mills <kevinm@mips.com>,
	 Catherine Moore <clm@codesourcery.com>,
	 Nathan Sidwell <nathan@codesourcery.com>,
	 Nathan Froyd <froydnj@codesourcery.com>
Subject: Re: [PATCH] MIPS: microMIPS ASE support
Date: Sat, 18 Dec 2010 10:26:00 -0000	[thread overview]
Message-ID: <87wrn79yum.fsf@firetop.home> (raw)
In-Reply-To: <alpine.DEB.1.10.1012152340131.4142@tp.orcam.me.uk> (Maciej	W. Rozycki's message of "Thu, 16 Dec 2010 11:16:23 +0000 (GMT)")

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
>> >  Only link variations of branches and jumps have a fixed-size delay slot 
>> > -- that's because the link register is set to a fixed offset from the 
>> > delay-slot instruction (either four as with JAL or two as with JALS).  Of 
>> > all such jumps and branches only JALX does not have a JALXS counterpart 
>> > (regrettably, as it would have made life of software much, much easier).
>> >
>> >  I've explained the meaning of 0 below -- it's unsafe to return this value 
>> > for a variable-size delay slot.
>> 
>> Hmm, I was thinking of the case where there was no branch _after_
>> the LUI, and where the instruction after the LUI could then become
>> the delay slot for a variable-length branch before the (deleted) LUI.
>> But yeah, I can see that 0 isn't correct if there is a branch immediately
>> after the LUI.
>
>  Well, if we have code like this:
>
> 	branch	...
> 	 LUI	...
> 	insn	[...]
>
> (where for the purpose of this consideration BRANCH may also be a jump) 
> then LUI cannot be entirely deleted and INSN moved into the slot of BRANCH 
> no matter if INSN is a branch or an computational instruction.  All we can 
> do in this case is to see if there is a corresponding BRANCHC instruction 
> and use it to swap BRANCH with and then delete the LUI if so, or otherwise 
> shrink the LUI to a 16-bit NOP if BRANCH permits or can be swapped with 
> BRANCHS to permit a 16-bit delay-slot instruction.  If neither is 
> possible, then the LUI is merely substituted with a 32-bit NOP (although 
> the effect is purely cosmetical in this case; perhaps we should just back 
> out).

Yeah, I see your point.  I was thinking that the code claims to "know"
that the LUI and "insn" are both part of the same load address.  So if
the branch was taken, the target of the LUI ought to be dead.  However,
I agree that (even though the code does seem to assume that to some extent)
the assumption is wrong.

E.g. you could have:

	beqz	$2,1f
	lui	$4,%hi(foo)	<-- A

	addiu	$4,$4,%lo(foo)	<-- B
	...
	jr      $31
2:	...
	lui	$4,%hi(foo)	<-- C
	...
1:	addiu   $4,$4,%lo(foo)	<-- D

In this case, the LO16 reloc for D might follow the HI16 reloc for C,
and the LO16 reloc for B might follow the HI16 reloc for A.  AIUI, we'd
consider relaxing A/B but not C/D.  In this case, turning A into a NOP
is wrong, because $4 is still live at D.  If you agree then...

>  Also with the recent update to LUI relaxation code I think we should 
> simply disallow the optimisation if a LUI is in a delay slot of an 
> unconditional branch -- we have no way to verify the corresponding LO16 
> reloc really belongs to this LUI instruction in that case.  This will let 
> us simplify code (which has become a little bit hairy by now IMO) a little 
> bit I would guess.  [FIXME]

...maybe it would be simpler to drop the optimisation if the LUI is any
kind of delay slot.  I think this would simply the code, and I don't think
we'd then need to check for branch relocs.  We'd just have *_norel-like
functions (although not called that any more) to check for every kind
of branch.

I obviously had a bit of a mental block when reviewing this delay slot
stuff, sorry.

Richard

  reply	other threads:[~2010-12-18 10:09 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-18 18:19 [PATCH] MIPS: microMIPS and MCU ASE instruction set support Maciej W. Rozycki
2010-05-23 21:38 ` Richard Sandiford
2010-05-24 22:25   ` Fu, Chao-Ying
2010-05-26 19:47     ` Richard Sandiford
2010-06-01 14:21   ` Maciej W. Rozycki
2010-06-01 14:39     ` Catherine Moore
2010-06-01 22:04     ` Richard Sandiford
2010-06-01 22:47     ` Fu, Chao-Ying
2010-06-05  9:17     ` Richard Sandiford
2010-07-26 10:56   ` [PATCH] MIPS: microMIPS ASE support Maciej W. Rozycki
2010-07-26 13:25     ` Nathan Froyd
2010-07-26 13:53       ` Maciej W. Rozycki
2010-07-26 19:03     ` Richard Sandiford
2010-12-07  1:13       ` Maciej W. Rozycki
2010-12-12 14:59         ` Richard Sandiford
2010-12-14 13:30           ` Maciej W. Rozycki
2010-12-14 14:51             ` Richard Sandiford
2010-12-16 11:54               ` Maciej W. Rozycki
2010-12-18 10:26                 ` Richard Sandiford [this message]
2010-12-14 17:56             ` Joseph S. Myers
2010-12-16 15:28               ` Maciej W. Rozycki
2010-12-17 20:56             ` Fu, Chao-Ying
2010-12-18 10:09               ` Richard Sandiford
2011-01-02 11:36         ` Richard Sandiford
2011-02-21 15:35           ` Maciej W. Rozycki
2011-02-22 20:12             ` Fu, Chao-Ying
2011-02-22 20:19             ` Fu, Chao-Ying
2011-02-24 10:46               ` Maciej W. Rozycki
2011-02-26 11:41                 ` Richard Sandiford
2011-02-28 16:41                   ` Maciej W. Rozycki
2011-02-26  0:00             ` Maciej W. Rozycki
2011-03-13  9:23               ` Richard Sandiford
2011-07-25  7:49                 ` Richard Sandiford
2011-07-26  2:01                   ` Maciej W. Rozycki
2011-07-29  0:58                     ` Maciej W. Rozycki
2011-07-29 11:30                       ` Richard Sandiford
2011-07-29 22:52                         ` Maciej W. Rozycki
2011-02-26 11:36             ` Richard Sandiford
2011-07-26 14:00               ` Maciej W. Rozycki
2010-05-26 20:19 ` [PATCH] MIPS: microMIPS and MCU ASE instruction set support Richard Sandiford
2010-05-27 21:39 ` Richard Sandiford

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87wrn79yum.fsf@firetop.home \
    --to=rdsandiford@googlemail.com \
    --cc=binutils@sourceware.org \
    --cc=clm@codesourcery.com \
    --cc=davidlau@mips.com \
    --cc=froydnj@codesourcery.com \
    --cc=fu@mips.com \
    --cc=ilie@mips.com \
    --cc=joseph@codesourcery.com \
    --cc=kevinm@mips.com \
    --cc=macro@codesourcery.com \
    --cc=nathan@codesourcery.com \
    --cc=rich@mips.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).