[PATCH]Clang and AArch64 backend patches to support sshll/ushll instructions

Tim Northover t.p.northover at gmail.com
Mon Aug 19 09:28:02 PDT 2013


Hi Hao,

> As I can't change vshll_n_s8(It must be Macro for the reason of checking the
> constant range), I use an local variable "a" in calling Macro vshll_n_s8 to
> forbid this conflict as following:

Ok, that sounds reasonable. But it shouldn't be called "a". The reason
most of these variables start with "__" is because the user is free to
write "#define a something_evil" before including arm_neon.h, but
identifiers beginning with double-underscore are reserved for the
compiler.

>         #define vshll_high_n_s8(a, __b) __extension__ ({ \
>           int8x16_t __a = (a); \
>           (int16x8_t)vshll_n_s8(vget_high_s8(a), __b); })

That's more worrying, you're evaluating "a" twice there. What if
someone wrote "vshll_high_n(some_func(), 2)" where some_func had
side-effects?

> However, I don't know whether the hack way is reasonable. If not, maybe we
> can just keep the current code and don't change it.

I'm not sure, I'm afraid. I think as it stands your patch is a slight
improvement, but that may change if it gets more complicated to deal
with those issues.

Cheers.

Tim.



More information about the llvm-commits mailing list