[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