uint32 as the return value for these functions seems like an odd and
arbitrary choice to me. What about adopting one of these options:
1) having them return the same type passed in
rationale: the user has chosen to work with a specific type, let's
preserve that type, as with arithmetic ops
2) having them return the int of the same width passed in
rationale: the user has chosen to work with a particular width and
ints tend to be easier to use in Chapel than uints due
to the C#-based signed/unsigned rules
3) having them all just return default 'int'
rationale: default ints are the default for most integral values
and tend to be flexible/work well
I think that, barring any additional information (like what the underlying
intrinsics themselves generate), I prefer the options in the order: 3, 2,
1. But that may just be because I hate working with uints in Chapel.
As long as I've opened up the patch, I'm also curious:
* Is there a strong reason not to support these operations on signed
integer types as well? One rationale might be that integers are less
of a bit vector than uints, but by that rationale, we shouldn't support
bitwise ops on them either (and I don't think we want to go down that
path).
* Do we fold param select statements in the compiler? If not, we should
create a future for that and you might get a performance improvement
in the meantime by using a conditional (which should get folded for a
param test like this).
* I also feel nervous about the "assumes..." comments in the .h file.
Could/should we assert that this is the case rather than just commenting
the assumption?
Thanks,
-Brad
On Tue, 20 May 2014, [email protected] wrote:
> Revision: 23418
> http://sourceforge.net/p/chapel/code/23418
> Author: kyle-b
> Date: 2014-05-20 22:12:15 +0000 (Tue, 20 May 2014)
> Log Message:
> -----------
> Tweak to BitOps module
>
> [Not reviewed]
>
> Changed the return type of clz/ctz/popcount to uint_32t from uint_64t. This
> change makes it a bit easier to use 32bit values with these functions.
>
> Modified Paths:
> --------------
> trunk/modules/standard/BitOps.chpl
> trunk/runtime/include/chpl-bitops.h
> trunk/test/modules/standard/BitOps/c-tests/clz.test.c
> trunk/test/modules/standard/BitOps/c-tests/ctz.test.c
> trunk/test/modules/standard/BitOps/c-tests/popcount.test.c
>
> This was sent by the SourceForge.net collaborative development platform, the
> world's largest Open Source development site.
>
>
> ------------------------------------------------------------------------------
> "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
> Instantly run your Selenium tests across 300+ browser/OS combos.
> Get unparalleled scalability from the best Selenium testing platform available
> Simple to use. Nothing to install. Get started now for free."
> http://p.sf.net/sfu/SauceLabs
> _______________________________________________
> Chapel-commits mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/chapel-commits
>
------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
Chapel-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/chapel-developers
