[PATCH] ARM & AArch64: merge the semantic checking of NEON intrinsics

Tim Northover t.p.northover at gmail.com
Tue Feb 11 11:24:33 PST 2014


Hi Ana,

> Did you verify this patch building all auto-generated headers?

It should have no effect on tests (I'll check that properly though) --
that's coming with later patches! Hopefully tomorrow.

I'll just mention the bigger problem I'm working on here, so that you
can read the rest with it in mind...

Right now, we've already got isA64 and isCrypto, and I'd need yet
another one to add the vcvtX intrinsics to 32-bit ARM (they're
v8-only, even on 32-bit, for obvious reasons). We want them predicated
(I'd say) by "#if __ARM_ARCH == 8". And I can't imagine this
problem-space is going to get simpler in future, so I'd like a
reasonably scalable solution).

> I mean did you modify \tools\clang\lib\Headers\Makefile with rules
> to build arm_neon_sema.h (-gen-arm-neon-sema) and arm_neon_test.h
> (-gen-arm-neon-test), in addition to arm_neon.h?

The -gen-arm-neon-sema is what I'm changing with this patch, so yes I
did test that. arm_neon.h is unaffected at this stage (again,
tomorrow).

> The check for uniqueness came from the fact AArch64 redefines some ARM intrinsics
> (it adds new types to the original type strings), so they do not appear more than once
> in the test list (coming from AArch64 and ARM intrinsics definitions). This was needed
> for generating the builtins list and I think for the tests too..

I recently merged the builtins list so that we don't need to have lots
of manual swapping back-and-forth between Aarch64::BI_XYZ and
ARM::BI_XYZ (everything lives under NEON::BI_XYZ now). This was based
on the observation that all intrinsics with common names also have
common semantics, so we don't actually need to separate them. I think
the same holds for error reporting, within limits.

As for the tests, I think there's got to be a better way, though I'm
still musing on exactly how, and that can happen after this change I
think.

Currently a lot of the information is duplicated (in odd ways) in
arm_neon.td. For example, even though the ARM "def" is responsible for
the definition int8x8_t variants in arm_neon.h, they have to be
present in the AArch64 "def" for the Sema checking to work.

This isn't ideal, it would be better to have just the new variants.
Take a look at the "with additional X, Y, Z" comments that have been
necessary in arm_neon.td just to point out what's been added for
example: first, that information should be obvious from the source
and, second, those comments (I've discovered) are frequently
inaccurate.

I think the best solution is to remove isA4 (and isCrypto) in favour
of some kind of string-based "ArchPredicate". We set this to
"__aarch64__" or "__ARM_FEATURE_CRYPTO", and everything gets generated
based on that. Then, if we can eliminate the duplication in
arm_neon.td, we just need to bracket the tests with the same predicate
and convince FileCheck to behave itself and we're done! Easy! What
could possibly go wrong!

Cheers.

Tim.



More information about the cfe-commits mailing list