5 ms·
My today’s WTF on the topic. The following C++ returns incorrect value when the argument is 8 or greater: inline uint64_t makeRemainderMask( ptrdiff_t miss
by Const-me 3y ago
My today’s WTF on the topic. The following C++ returns incorrect value when the argument is 8 or greater:
inline uint64_t makeRemainderMask( ptrdiff_t missingLanes )
{
// This is not a branch, compiles to conditional move
missingLanes = std::max( missingLanes, (ptrdiff_t)0 );
// Make a mask of 8 bytes
// No need to clip for missingLanes <= 8 because the shift is already good, results in zero
uint64_t mask = ~(uint64_t)0;
mask >>= missingLanes * 8;
return mask;
}
TIL the language standard defines right shift operator for unsigned types in a weird way, making so for uint64_t argument, a >> b is equal to a >> ( b % 64 ). I expected zero on the output for b >= 64, not that.
- dzaima 3y agoC++ doesn't even define it as that; it's actually just undefined behavior to shift by any amount greater than or equal to the width!
- gosub100 3y agoThats not on the topic though. Do you think because two languages share a common letter in their name that they are equivocal?
- trealira 3y agoThey said C++, but aside from the line containing "std::max," they wrote C code, and what they said applies to C as well.
- gosub100 3y agothen why not post a standard C example?
- Const-me 3y agoIf I would rework that snippet to be in C instead of C++, that code would no longer be my today’s WTF. Also, IMO `std::max` is more readable than the equivalent conditional operator.
- gosub100 3y agoIts irrelevant and a different language. might as well post ocaml or julia
- Const-me 3y agoVery relevant. Apart from a few minor exceptions, C++ is a superset of C i.e. almost any C code can be compiled as C++ language, and will result in a correct program.
- ryao 3y agoTry compiling C code using union types for their intended type punning purpose as C++. It will compile, but bad things could happen. :/
- ryao 3y agoIf the C++ compiler takes it, it is C++ code. The fact that it is almost valid C code does not change that. The reactions a number of people have to C++ code that is also valid C code or close to being it are ridiculous. Some times, they even deny C++ code is C++ code. :/
- trealira 3y agoI was just saying that it's not off-topic. I don't really understand what you're trying to say otherwise.
- ez667 3y agodo you think that you know what "equivocal" means ?
- ksherlock 3y agoCPUs handle excessively large shifts in implementation defined ways. Some return 0. Some trap. x86 masks the right hand side with & 63 (for a 64-bit shift) so shifting by 65 is equivalent to shifting by 1. In C and C++ it's undefined behavior (Really it should be implementation defined)
- dzaima 3y agoAnother different shift implementation is in ARM NEON, where, for a non-constant shift amount, there's only a shift-left, and that with a negative shift amount gets you right shift; thus, were it forced to choose only one behavior, either scalar or vector shifts would get worse.
- Const-me 3y agoInteresting, thanks. BTW, I fixed like that: uint64_t mask = -( missingLanes < 8 ); mask >>= missingLanes * 8; I only care about the AMD64 because the surrounding code uses AVX2 intrinsics, i.e. my new version is hopefully good enough. I’ve tried conditional operator to not rely on these details, however VC++ failed to generate fast code from them. It emit branches, and that code is kinda performance critical.
- sweetjuly 3y agoRISC-V is even more fun where they shift based on shamt[4:0] (or [5:0] in XLEN=64 mode). So, if you do something like: li t0, 128 li t1, 1 sll t2, t1, t0 You'll get t2 := 1 since t0[5:0] == 6'b0 (i.e. no shift). It's a very sensible solution IMO if you don't have trapping arithmetic since you don't have to do anything special to handle illegal shifts, it just works.
- ericbarrett 3y agoI really like Rust's answer in this space. Check out all the useful methods on the u64 type that can make this kind of code explicit: https://doc.rust-lang.org/std/primitive.u64.html https://doc.rust-lang.org/std/primitive.u64.html
- not2b 3y agoNo, it doesn't. Shifting by a wider amount than the width of the type, or an equal amount, is undefined in C and C++. That was done because different processors handled the case differently, meaning that any specification would give one processor a substantial time penalty compared to the competition. So the committee gave up on obtaining agreement and called it undefined.
- bxparks 3y agoBut why did they make it Undefined Behavior instead of Implementation Defined? I love the C language, but there are now so many UBs in the language, it is painful to use.
- ryao 3y agoIf they did, we would have two subtly different dialects of C for cyclic shift and zeroing shift, and possibly even worse, combinations of the two. That said, I am curious what the official answer is myself.
- ryao 3y agoThis happens in C too. Static analyzers tend to be good at catching this.