[1/3] atrac3plusdsp: Use LOCAL_ALIGNED for stack variables
diff mbox

Message ID 20170329110232.22383-1-martin@martin.st
State New
Headers show

Commit Message

Martin Storsjö March 29, 2017, 11:02 a.m. 
This fixes warnings like these with ARMCC:
alignment for an auto object may not be larger than 8
---
 libavcodec/atrac3plusdsp.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Diego Biurrun March 29, 2017, 5:53 p.m. | #1 
On Wed, Mar 29, 2017 at 02:02:30PM +0300, Martin Storsjö wrote:
> --- a/libavcodec/atrac3plusdsp.c
> +++ b/libavcodec/atrac3plusdsp.c
> @@ -177,12 +177,14 @@ static void waves_synth(Atrac3pWaveSynthParams *synth_param,
>  void ff_atrac3p_generate_tones(Atrac3pChanUnitCtx *ch_unit, AVFloatDSPContext *fdsp,
>                                 int ch_num, int sb, float *out)
>  {
> -    DECLARE_ALIGNED(32, float, wavreg1)[128] = { 0 };
> -    DECLARE_ALIGNED(32, float, wavreg2)[128] = { 0 };
> +    LOCAL_ALIGNED_32(float, wavreg1, [128]);
> +    LOCAL_ALIGNED_32(float, wavreg2, [128]);
>      int i, reg1_env_nonzero, reg2_env_nonzero;
>      Atrac3pWavesData *tones_now  = &ch_unit->channels[ch_num].tones_info_prev[sb];
>      Atrac3pWavesData *tones_next = &ch_unit->channels[ch_num].tones_info[sb];
>  
> +    memset(wavreg1, 0, sizeof(float) * 128);
> +    memset(wavreg2, 0, sizeof(float) * 128);

We have to resort to these manual memsets in many other places. I think
the proper solution would be to add LOCAL_ALIGNEDZ macros that include
a zero initialization.

Diego
Hendrik Leppkes March 29, 2017, 6:24 p.m. | #2 
On Wed, Mar 29, 2017 at 7:53 PM, Diego Biurrun <diego@biurrun.de> wrote:
> On Wed, Mar 29, 2017 at 02:02:30PM +0300, Martin Storsjö wrote:
>> --- a/libavcodec/atrac3plusdsp.c
>> +++ b/libavcodec/atrac3plusdsp.c
>> @@ -177,12 +177,14 @@ static void waves_synth(Atrac3pWaveSynthParams *synth_param,
>>  void ff_atrac3p_generate_tones(Atrac3pChanUnitCtx *ch_unit, AVFloatDSPContext *fdsp,
>>                                 int ch_num, int sb, float *out)
>>  {
>> -    DECLARE_ALIGNED(32, float, wavreg1)[128] = { 0 };
>> -    DECLARE_ALIGNED(32, float, wavreg2)[128] = { 0 };
>> +    LOCAL_ALIGNED_32(float, wavreg1, [128]);
>> +    LOCAL_ALIGNED_32(float, wavreg2, [128]);
>>      int i, reg1_env_nonzero, reg2_env_nonzero;
>>      Atrac3pWavesData *tones_now  = &ch_unit->channels[ch_num].tones_info_prev[sb];
>>      Atrac3pWavesData *tones_next = &ch_unit->channels[ch_num].tones_info[sb];
>>
>> +    memset(wavreg1, 0, sizeof(float) * 128);
>> +    memset(wavreg2, 0, sizeof(float) * 128);
>
> We have to resort to these manual memsets in many other places. I think
> the proper solution would be to add LOCAL_ALIGNEDZ macros that include
> a zero initialization.
>

That wouldn't be possible with declarations before statements rules, would it?

- Hendrik
Diego Biurrun March 30, 2017, 10:27 a.m. | #3 
On Wed, Mar 29, 2017 at 08:24:27PM +0200, Hendrik Leppkes wrote:
> On Wed, Mar 29, 2017 at 7:53 PM, Diego Biurrun <diego@biurrun.de> wrote:
> > On Wed, Mar 29, 2017 at 02:02:30PM +0300, Martin Storsjö wrote:
> >> --- a/libavcodec/atrac3plusdsp.c
> >> +++ b/libavcodec/atrac3plusdsp.c
> >> @@ -177,12 +177,14 @@ static void waves_synth(Atrac3pWaveSynthParams *synth_param,
> >>  void ff_atrac3p_generate_tones(Atrac3pChanUnitCtx *ch_unit, AVFloatDSPContext *fdsp,
> >>                                 int ch_num, int sb, float *out)
> >>  {
> >> -    DECLARE_ALIGNED(32, float, wavreg1)[128] = { 0 };
> >> -    DECLARE_ALIGNED(32, float, wavreg2)[128] = { 0 };
> >> +    LOCAL_ALIGNED_32(float, wavreg1, [128]);
> >> +    LOCAL_ALIGNED_32(float, wavreg2, [128]);
> >>      int i, reg1_env_nonzero, reg2_env_nonzero;
> >>      Atrac3pWavesData *tones_now  = &ch_unit->channels[ch_num].tones_info_prev[sb];
> >>      Atrac3pWavesData *tones_next = &ch_unit->channels[ch_num].tones_info[sb];
> >>
> >> +    memset(wavreg1, 0, sizeof(float) * 128);
> >> +    memset(wavreg2, 0, sizeof(float) * 128);
> >
> > We have to resort to these manual memsets in many other places. I think
> > the proper solution would be to add LOCAL_ALIGNEDZ macros that include
> > a zero initialization.
> 
> That wouldn't be possible with declarations before statements rules, would it?

Huh?

Diego
Martin Storsjö March 30, 2017, 11:07 a.m. | #4 
On Wed, 29 Mar 2017, Hendrik Leppkes wrote:

> On Wed, Mar 29, 2017 at 7:53 PM, Diego Biurrun <diego@biurrun.de> wrote:
>> On Wed, Mar 29, 2017 at 02:02:30PM +0300, Martin Storsjö wrote:
>>> --- a/libavcodec/atrac3plusdsp.c
>>> +++ b/libavcodec/atrac3plusdsp.c
>>> @@ -177,12 +177,14 @@ static void waves_synth(Atrac3pWaveSynthParams *synth_param,
>>>  void ff_atrac3p_generate_tones(Atrac3pChanUnitCtx *ch_unit, AVFloatDSPContext *fdsp,
>>>                                 int ch_num, int sb, float *out)
>>>  {
>>> -    DECLARE_ALIGNED(32, float, wavreg1)[128] = { 0 };
>>> -    DECLARE_ALIGNED(32, float, wavreg2)[128] = { 0 };
>>> +    LOCAL_ALIGNED_32(float, wavreg1, [128]);
>>> +    LOCAL_ALIGNED_32(float, wavreg2, [128]);
>>>      int i, reg1_env_nonzero, reg2_env_nonzero;
>>>      Atrac3pWavesData *tones_now  = &ch_unit->channels[ch_num].tones_info_prev[sb];
>>>      Atrac3pWavesData *tones_next = &ch_unit->channels[ch_num].tones_info[sb];
>>>
>>> +    memset(wavreg1, 0, sizeof(float) * 128);
>>> +    memset(wavreg2, 0, sizeof(float) * 128);
>>
>> We have to resort to these manual memsets in many other places. I think
>> the proper solution would be to add LOCAL_ALIGNEDZ macros that include
>> a zero initialization.
>>
>
> That wouldn't be possible with declarations before statements rules, would it?

You should be able to use normal (= { 0 }) zero initialization for the 
array used within LOCAL_ALIGNED if you modify the macros, instead of 
having a separate memset for it. It does requires a bit of wrangling of 
the macros though.

// Martin

Patch
diff mbox 

diff --git a/libavcodec/atrac3plusdsp.c b/libavcodec/atrac3plusdsp.c
index 468f098383..c1a54ae266 100644
--- a/libavcodec/atrac3plusdsp.c
+++ b/libavcodec/atrac3plusdsp.c
@@ -177,12 +177,14 @@  static void waves_synth(Atrac3pWaveSynthParams *synth_param,
 void ff_atrac3p_generate_tones(Atrac3pChanUnitCtx *ch_unit, AVFloatDSPContext *fdsp,
                                int ch_num, int sb, float *out)
 {
-    DECLARE_ALIGNED(32, float, wavreg1)[128] = { 0 };
-    DECLARE_ALIGNED(32, float, wavreg2)[128] = { 0 };
+    LOCAL_ALIGNED_32(float, wavreg1, [128]);
+    LOCAL_ALIGNED_32(float, wavreg2, [128]);
     int i, reg1_env_nonzero, reg2_env_nonzero;
     Atrac3pWavesData *tones_now  = &ch_unit->channels[ch_num].tones_info_prev[sb];
     Atrac3pWavesData *tones_next = &ch_unit->channels[ch_num].tones_info[sb];
 
+    memset(wavreg1, 0, sizeof(float) * 128);
+    memset(wavreg2, 0, sizeof(float) * 128);
     /* reconstruct full envelopes for both overlapping regions
      * from truncated bitstream data */
     if (tones_next->pend_env.has_start_point &&
@@ -599,8 +601,8 @@  void ff_atrac3p_ipqf(FFTContext *dct_ctx, Atrac3pIPQFChannelCtx *hist,
                      const float *in, float *out)
 {
     int i, s, sb, t, pos_now, pos_next;
-    DECLARE_ALIGNED(32, float, idct_in)[ATRAC3P_SUBBANDS];
-    DECLARE_ALIGNED(32, float, idct_out)[ATRAC3P_SUBBANDS];
+    LOCAL_ALIGNED_32(float, idct_in,  [ATRAC3P_SUBBANDS]);
+    LOCAL_ALIGNED_32(float, idct_out, [ATRAC3P_SUBBANDS]);
 
     memset(out, 0, ATRAC3P_FRAME_SAMPLES * sizeof(*out));