Hi James,

On 06/02/19 17:27, James Greenhalgh wrote:
On Wed, Feb 06, 2019 at 07:52:42AM -0600, Kyrill Tkachov wrote:
[resending with patch compressed]

Hi all,

We're somewhat inconsistent in arm_neon.h when it comes to using the 
implementation namespace for local
identifiers. This means things like:
#define hash_abcd 0
#define hash_e 1
#define wk 2

#include "arm_neon.h"

uint32x4_t
foo (uint32x4_t a, uint32_t b, uint32x4_t c)
{
    return vsha1cq_u32 (a, b, c);
}

don't compile.
This patch fixes these issues throughout the whole of arm_neon.h
Bootstrapped and tested on aarch64-none-linux-gnu.
The advsimd-intrinsics.exp tests pass just fine.

Don't feel sorry for me having to write the ChangeLog. ./contrib/mklog.pl 
automated the whole thing.

Ok for trunk?
I assume you've just run some simple sed over this file.

I'd rather review them than the patch; what were they?

I... errr.. I did it semi-manually in my graphical editor by doing 
search-and-replace for things like
" a)" -> " __a)" , " a," -> " __a," , ")a " -> ")__a " etc.

Same for 'b', 'c', 'd', 'r', 'temp', 'val', 'result' and any other offending 
identifier I could spot.
I left things like the "val" in:
typedef struct int8x8x2_t
{
  int8x8_t val[2];
} int8x8x2_t;

alone, but there were some uses of the identifier "val" as an intrinsic parameter so I 
converted those to "__val"

If any of the changes are invalid (changing parameter name in intrinsic, but 
not its use for example) then any testcase using arm_neon.h breaks,
even if the intrinsic in question is not used, so I expect the testing enforces 
the correctness.

Thanks,
Kyrill


James

2019-02-06  Kyrylo Tkachov  <kyrylo.tkac...@arm.com>

      * config/aarch64/arm_neon.h (vaba_s8): Use __ in identifiers
      consistenly.
<snip a great example for the next cauldron discussion about the
   value of ChangeLogs>

Reply via email to