Re: [PATCH] ARM: vfp: Fix up exception location in Thumb mode

[Catalin Marinas]

On Sun, 2011-01-23 at 15:51 +0000, Russell King - ARM Linux wrote:
> On Sun, Jan 16, 2011 at 11:49:21AM +0000, Catalin Marinas wrote:
> > On Saturday, 15 January 2011, Russell King - ARM Linux
> > <linux@xxxxxxxxxxxxxxxx> wrote:
> > > It's a reveq, so I thought we should cover all the instructions with
> > > an 'eq' conditional for thumb.
> >
> > If the it instruction doesn't cover all instructions, gas generates
> > some more its. But in this case, for little endian, the it instruction
> > covers more since reveq isn't included and having the beq not last in
> > the block I think is unpredictable. If you really want to optimise the
> > big endian case not to have an additional it generated by gas, you can
> > write ittt so that beq is included with little endian but not with big
> > endian. I wouldn't bother much for an extra it anyway.
> 
> I think the itttt is correct.  Unless you wish to illustrate why you
> think it's wrong by pasting the code and showing why you think the
> beq isn't the last instruction...

With your patch applied (visually), the code becomes (removed the
comment before beq):

	tst	r3, #PSR_T_BIT			@ Thumb mode?
	itttt	eq				@ explicit IT needed for the 1f label
	subeq	r4, r2, #4			@ ARM instr at LR - 4
1:	ldreqt	r0, [r4]
#ifdef CONFIG_CPU_ENDIAN_BE8
	reveq	r0, r0				@ little endian instruction
#endif
	beq	call_fpe

The little endian case only has 3 conditional instructions:

	subeq	r4, r2, #4			@ ARM instr at LR - 4
1:	ldreqt	r0, [r4]
	beq	call_fpe

but you add itttt (if-then-then-then-then) which expects 4 conditional
instructions, IOW beq is no longer the last. So cutting a 't' would sort
it out (unless I misread your patch).

> > >         tst     r3, #PSR_T_BIT
> > >         subeq   r4, r2, #4
> > > 1:      ldreqt  r0, [r4]
> > >         reveq   r0, r0
> > >         beq     call_fpe
> >
> > You can have the T bit set but the instruction a 32-bit Thumb in which
> > case r2 is in the middle of such instruction rather than the next.
> > Unless you only refer to the ARM mode, in which case the comment is
> > fine.
> 
> So?  I'm confused why you're making a mountain out of apparantly
> nothing.

No issue really, the comment can stay as you wrote it (I don't read them
anyway :)).

-- 
Catalin


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/