| Message ID | 1479456399-18568-1-git-send-email-martin@martin.st |
|---|---|
| State | Rejected |
| Headers | show |
On 2016-11-18 10:06:38 +0200, Martin Storsjö wrote: > This allows using the cbz/cbnz instruction in thumb mode, for the > cases where the branch target is close enough. > --- > libavutil/arm/asm.S | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/libavutil/arm/asm.S b/libavutil/arm/asm.S > index 4ac0ea2..e66c34c 100644 > --- a/libavutil/arm/asm.S > +++ b/libavutil/arm/asm.S > @@ -326,6 +326,18 @@ T strh \rt, [\rn] > T sub \rn, \rn, \rm > .endm > > +#if !CONFIG_THUMB > +.macro cbz rd, target > + cmp \rd, #0 > + beq \target > +.endm > + > +.macro cbnz rd, target > + cmp \rd, #0 > + bne \target > +.endm > +#endif I'm not convinced this is a good idea, the thumb instruction works only with r0-r7 and has a very limited range while the arm version has no such limitations. I.e. unless one tests thumb explicitly it's very easy to break thumb. For the few places where it is useful 'T' and 'A' prefixed instructions are imo the better way. Using instructions setting APSR flags are preferable to cbn?z. This differs on aarch64 since there only a some instructions can set flags. Also I optimized one intended use away. Janne
On Fri, 18 Nov 2016, Janne Grunau wrote: > On 2016-11-18 10:06:38 +0200, Martin Storsjö wrote: >> This allows using the cbz/cbnz instruction in thumb mode, for the >> cases where the branch target is close enough. >> --- >> libavutil/arm/asm.S | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/libavutil/arm/asm.S b/libavutil/arm/asm.S >> index 4ac0ea2..e66c34c 100644 >> --- a/libavutil/arm/asm.S >> +++ b/libavutil/arm/asm.S >> @@ -326,6 +326,18 @@ T strh \rt, [\rn] >> T sub \rn, \rn, \rm >> .endm >> >> +#if !CONFIG_THUMB >> +.macro cbz rd, target >> + cmp \rd, #0 >> + beq \target >> +.endm >> + >> +.macro cbnz rd, target >> + cmp \rd, #0 >> + bne \target >> +.endm >> +#endif > > I'm not convinced this is a good idea, the thumb instruction works only > with r0-r7 and has a very limited range while the arm version has no > such limitations. I.e. unless one tests thumb explicitly it's very easy > to break thumb. For the few places where it is useful 'T' and 'A' > prefixed instructions are imo the better way. Using instructions setting > APSR flags are preferable to cbn?z. This differs on aarch64 since there > only a some instructions can set flags. > > Also I optimized one intended use away. Fair enough. Yes, the limited range makes it impractical in many cases, I had expected to use it in the vp9 loop filter. I also noticed now that gas-preprocessor doesn't handle cbz/cbnz translation of branch targets (1f etc) for armasm yet, we apparently don't use it anywhere yet. Patch dropped then. // Martin
diff --git a/libavutil/arm/asm.S b/libavutil/arm/asm.S index 4ac0ea2..e66c34c 100644 --- a/libavutil/arm/asm.S +++ b/libavutil/arm/asm.S @@ -326,6 +326,18 @@ T strh \rt, [\rn] T sub \rn, \rn, \rm .endm +#if !CONFIG_THUMB +.macro cbz rd, target + cmp \rd, #0 + beq \target +.endm + +.macro cbnz rd, target + cmp \rd, #0 + bne \target +.endm +#endif + #if HAVE_VFP_ARGS ELF .eabi_attribute 28, 1 # define VFP