Patchwork [SH] PR 54685 - unsigned int comparison with 0x7FFFFFFF

login
register
mail settings
Submitter Oleg Endo
Date Oct. 8, 2012, 2:15 a.m.
Message ID <1349662554.21984.32.camel@yam-132-YW-E178-FTW>
Download mbox | patch
Permalink /patch/189880/
State New
Headers show

Comments

Oleg Endo - Oct. 8, 2012, 2:15 a.m.
On Mon, 2012-10-08 at 03:53 +0200, Oleg Endo wrote:
> On Mon, 2012-10-08 at 09:45 +0900, Kaz Kojima wrote:
> > Oleg Endo <oleg.endo@t-online.de> wrote:
> > > The attached patch improves comparisons such as
> > > 'unsigned int <= 0x7FFFFFFF' on SH.
> > > As mentioned in the PR, for some reason, those comparisons do not go
> > > through the cstore expander.  As a consequence the comparison doesn't
> > > get the chance to be canonicalized by the target code and ends up as
> > > '(~x) >> 31'.
> > > I've not investigated this further and just fixed the symptoms on SH.  I
> > > don't know whether it's also an issue on other targets.
> > > 
> > > Tested on rev 192142 with
> > > make -k check RUNTESTFLAGS="--target_board=sh-sim
> > > \{-m2/-ml,-m2/-mb,-m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}"
> > > 
> > > and no new failures.
> > > OK?
> > 
> > I've run CSiBE with and without the patch for sh4-unknown-linux-gnu
> > at -O2.  Only one difference in the resulted sizes: jpeg-6b/jcphuff
> > increases 5336 bytes to 5340 bytes with the patch.  Could you look
> > into it?
> 
> Yep, that's actually the only place in the CSiBE set where this case
> hits.  The function in question is encode_mcu_AC_refine.  The increase
> seems to be due to different register allocation and different spill
> code :T
> I've attached the asm diff.

I've just checked this against current trunk (rev 192193).  The problem
seems to be gone.  There's also a total decrease of 152 bytes on this
file without the patch.  So it seems it was a different issue.  The diff
is now:


Cheers,
Oleg
Kaz Kojima - Oct. 8, 2012, 2:32 a.m.
Oleg Endo <oleg.endo@t-online.de> wrote:
> I've just checked this against current trunk (rev 192193).  The problem
> seems to be gone.  There's also a total decrease of 152 bytes on this
> file without the patch.  So it seems it was a different issue.  The diff
> is now:

Thanks for looking into it quickly.  The patch is OK.

Regards,
	kaz

Patch

--- CSiBE/m4-single-ml-O2/jpeg-6b/jcphuff_.s
+++ CSiBE/m4-single-ml-O2/jpeg-6b/jcphuff.s
@@ -2626,13 +2626,12 @@ 
 	add	r0,r0
 	mov.l	r3,@(24,r13)
 	bf/s	.L502
-	mov.w	@(r0,r1),r11
+	mov.w	@(r0,r1),r7
 	mov.l	@(12,r15),r0
-	not	r11,r11
-	shll	r11
+	cmp/pz	r7
 	mov.l	@(12,r15),r10
+	movt	r11
 	neg	r0,r2
-	movt	r11
 	add	#23,r2
 	shld	r2,r11
 	add	#1,r10