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

Reply via email to