compnerd added inline comments.
================ Comment at: clang/lib/Headers/arm_acle.h:150 +__clsl(unsigned long __t) { +#if __SIZEOF_LONG__ == 4 + return __builtin_arm_cls(__t); ---------------- vhscampos wrote: > compnerd wrote: > > I don't see a pattern match for the `cls64` on ARM32, would that not fail > > to lower? > Yes. However, for now, I am not enabling support for `cls64` on ARM32 as it > is not done yet. Is the difference not just the parameter type? I think that implementing it should be a trivial change to the existing implementation. Is there a reason that you are not implementing that? ================ Comment at: clang/lib/Headers/arm_acle.h:155 +#endif +} + ---------------- vhscampos wrote: > compnerd wrote: > > Should we have a `__clsll` extension, otherwise these two are the same in > > LLP64? I'm thinking about the LLP64 environments, where `long` and `long > > long` are different (32-bit vs 64-bit). > ACLE does provide a `long long` version of `cls` called `__clsll`. But since > the support for `cls64` on Arm32 is not done yet, I decided not to write > support for `__clsll`. If I did, it would work for 64-bit but not for 32-bit. > > Please let me know what you think. clang supports Windows where `long` is 4-bytes even on 64-bit targets, and this means that this doesn't work for that target. I think that we need to add `__clsll` so that 64-bit ARM at least is covered. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69250/new/ https://reviews.llvm.org/D69250 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits