x86: Fix constraints for decode_significance*_x86

Message ID 1324929764-50147-1-git-send-email-martin@martin.st
State Committed
Commit 676a9ee1d2f88e320a00d56c47bdac86881bc75b
Headers show

Commit Message

Martin Storsjö Dec. 26, 2011, 8:02 p.m.
Originally, prior to 8742a4ff8, the caller code was compiled
within this condition:

ARCH_X86 && HAVE_7REGS && HAVE_EBX_AVAILABLE && !defined(BROKEN_RELOCATIONS)

Since HAVE_7REGS is defined as
(ARCH_X86_64 || (HAVE_EBX_AVAILABLE && HAVE_EBP_AVAILABLE))
the subcondition HAVE_7REGS && HAVE_EBX_AVAILABLE is equal
to HAVE_7REGS (for 32 bit at least). The correct simplification
of the original condition thus is HAVE_7REGS, not
HAVE_EBX_AVAILABLE.

This fixes compilation in some cases where HAVE_EBP_AVAILABLE = 0
and HAVE_EBX_AVAILABLE = 1.

---

Reinhard, does this fix compilation in your cases?

 libavcodec/h264_cabac.c    |    2 +-
 libavcodec/x86/h264_i386.h |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Reinhard Tartler Dec. 26, 2011, 10:46 p.m. | #1
On Mo, Dez 26, 2011 at 21:02:44 (CET), Martin Storsjö wrote:

> Originally, prior to 8742a4ff8, the caller code was compiled
> within this condition:
>
> ARCH_X86 && HAVE_7REGS && HAVE_EBX_AVAILABLE && !defined(BROKEN_RELOCATIONS)
>
> Since HAVE_7REGS is defined as
> (ARCH_X86_64 || (HAVE_EBX_AVAILABLE && HAVE_EBP_AVAILABLE))
> the subcondition HAVE_7REGS && HAVE_EBX_AVAILABLE is equal
> to HAVE_7REGS (for 32 bit at least). The correct simplification
> of the original condition thus is HAVE_7REGS, not
> HAVE_EBX_AVAILABLE.
>
> This fixes compilation in some cases where HAVE_EBP_AVAILABLE = 0
> and HAVE_EBX_AVAILABLE = 1.

This commit message makes sense to me. Thanks!

> ---
>
> Reinhard, does this fix compilation in your cases?

Yes, this patch does allow mplayer (svn) to compile again.


>  libavcodec/h264_cabac.c    |    2 +-
>  libavcodec/x86/h264_i386.h |    4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/libavcodec/h264_cabac.c b/libavcodec/h264_cabac.c
> index a96f52e..feadf49 100644
> --- a/libavcodec/h264_cabac.c
> +++ b/libavcodec/h264_cabac.c
> @@ -1656,7 +1656,7 @@ decode_cabac_residual_internal(H264Context *h, DCTELEM *block,
>              index[coeff_count++] = last;\
>          }
>          const uint8_t *sig_off = significant_coeff_flag_offset_8x8[MB_FIELD];
> -#if ARCH_X86 && HAVE_EBX_AVAILABLE && !defined(BROKEN_RELOCATIONS)
> +#if ARCH_X86 && HAVE_7REGS && !defined(BROKEN_RELOCATIONS)
>          coeff_count= decode_significance_8x8_x86(CC, significant_coeff_ctx_base, index,
>                                                   last_coeff_ctx_base, sig_off);
>      } else {
> diff --git a/libavcodec/x86/h264_i386.h b/libavcodec/x86/h264_i386.h
> index 6cd81fe..e195e04 100644
> --- a/libavcodec/x86/h264_i386.h
> +++ b/libavcodec/x86/h264_i386.h
> @@ -36,7 +36,7 @@
>  
>  //FIXME use some macros to avoid duplicating get_cabac (cannot be done yet
>  //as that would make optimization work hard)
> -#if HAVE_EBX_AVAILABLE && !defined(BROKEN_RELOCATIONS)
> +#if HAVE_7REGS && !defined(BROKEN_RELOCATIONS)
>  static int decode_significance_x86(CABACContext *c, int max_coeff,
>                                     uint8_t *significant_coeff_ctx_base,
>                                     int *index, x86_reg last_off){
> @@ -144,6 +144,6 @@ static int decode_significance_8x8_x86(CABACContext *c,
>      );
>      return coeff_count;
>  }
> -#endif /* HAVE_EBX_AVAILABLE && !defined(BROKEN_RELOCATIONS) */
> +#endif /* HAVE_7REGS && !defined(BROKEN_RELOCATIONS) */
>  
>  #endif /* AVCODEC_X86_H264_I386_H */

Cheers!
Mans Rullgard Dec. 27, 2011, 11:43 a.m. | #2
Martin Storsjö <martin@martin.st> writes:

> Originally, prior to 8742a4ff8, the caller code was compiled
> within this condition:
>
> ARCH_X86 && HAVE_7REGS && HAVE_EBX_AVAILABLE && !defined(BROKEN_RELOCATIONS)
>
> Since HAVE_7REGS is defined as
> (ARCH_X86_64 || (HAVE_EBX_AVAILABLE && HAVE_EBP_AVAILABLE))
> the subcondition HAVE_7REGS && HAVE_EBX_AVAILABLE is equal
> to HAVE_7REGS (for 32 bit at least). The correct simplification
> of the original condition thus is HAVE_7REGS, not
> HAVE_EBX_AVAILABLE.
>
> This fixes compilation in some cases where HAVE_EBP_AVAILABLE = 0
> and HAVE_EBX_AVAILABLE = 1.
>
> ---
>
> Reinhard, does this fix compilation in your cases?
>
>  libavcodec/h264_cabac.c    |    2 +-
>  libavcodec/x86/h264_i386.h |    4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/libavcodec/h264_cabac.c b/libavcodec/h264_cabac.c
> index a96f52e..feadf49 100644
> --- a/libavcodec/h264_cabac.c
> +++ b/libavcodec/h264_cabac.c
> @@ -1656,7 +1656,7 @@ decode_cabac_residual_internal(H264Context *h, DCTELEM *block,
>              index[coeff_count++] = last;\
>          }
>          const uint8_t *sig_off = significant_coeff_flag_offset_8x8[MB_FIELD];
> -#if ARCH_X86 && HAVE_EBX_AVAILABLE && !defined(BROKEN_RELOCATIONS)
> +#if ARCH_X86 && HAVE_7REGS && !defined(BROKEN_RELOCATIONS)
>          coeff_count= decode_significance_8x8_x86(CC, significant_coeff_ctx_base, index,
>                                                   last_coeff_ctx_base, sig_off);

7REGS is a bit weird here since the asm only has 6 register operands
(including an ecx clobber).  Presumably the memory operands need a
register as well.  In any case, there is no explicit mention of ebx in
the asm, so that condition is wrong for sure.

Patch OK if it fixes things.

Patch

diff --git a/libavcodec/h264_cabac.c b/libavcodec/h264_cabac.c
index a96f52e..feadf49 100644
--- a/libavcodec/h264_cabac.c
+++ b/libavcodec/h264_cabac.c
@@ -1656,7 +1656,7 @@  decode_cabac_residual_internal(H264Context *h, DCTELEM *block,
             index[coeff_count++] = last;\
         }
         const uint8_t *sig_off = significant_coeff_flag_offset_8x8[MB_FIELD];
-#if ARCH_X86 && HAVE_EBX_AVAILABLE && !defined(BROKEN_RELOCATIONS)
+#if ARCH_X86 && HAVE_7REGS && !defined(BROKEN_RELOCATIONS)
         coeff_count= decode_significance_8x8_x86(CC, significant_coeff_ctx_base, index,
                                                  last_coeff_ctx_base, sig_off);
     } else {
diff --git a/libavcodec/x86/h264_i386.h b/libavcodec/x86/h264_i386.h
index 6cd81fe..e195e04 100644
--- a/libavcodec/x86/h264_i386.h
+++ b/libavcodec/x86/h264_i386.h
@@ -36,7 +36,7 @@ 
 
 //FIXME use some macros to avoid duplicating get_cabac (cannot be done yet
 //as that would make optimization work hard)
-#if HAVE_EBX_AVAILABLE && !defined(BROKEN_RELOCATIONS)
+#if HAVE_7REGS && !defined(BROKEN_RELOCATIONS)
 static int decode_significance_x86(CABACContext *c, int max_coeff,
                                    uint8_t *significant_coeff_ctx_base,
                                    int *index, x86_reg last_off){
@@ -144,6 +144,6 @@  static int decode_significance_8x8_x86(CABACContext *c,
     );
     return coeff_count;
 }
-#endif /* HAVE_EBX_AVAILABLE && !defined(BROKEN_RELOCATIONS) */
+#endif /* HAVE_7REGS && !defined(BROKEN_RELOCATIONS) */
 
 #endif /* AVCODEC_X86_H264_I386_H */