Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
clang/lib/Headers/avx2intrin.h | ||
---|---|---|
3474 | LoadXX is the syntax in the gather intrinsics, e.g. _mm_mask_i32gather_pd. I'd prefer to be consistent. |
clang/lib/Headers/avx2intrin.h | ||
---|---|---|
3474 | I think the problem here is the measurement is easily confusing. |
clang/lib/Headers/avx2intrin.h | ||
---|---|---|
3474 | Well, the pseudo-code is clearly not C. If you look at the gather code, it computes a byte address using an offset multiplied by an explicit scale factor. I am doing exactly the same here. The syntax MEM[__X+j:j] is mixing a byte address with a bit offset, which I think is more confusing. To be fully consistent, using [] with bit offsets only, it should be k := __X*8 + i*32 result[j+31:j] := MEM[k+31:k] which I think obscures more than it explains. |
clang/lib/Headers/avx2intrin.h | ||
---|---|---|
3474 | Yeah, it's not C code here. But we are easy to fall into C concepts, e.g., why assuming __X is measuring in bytes? |
clang/lib/Headers/avx2intrin.h | ||
---|---|---|
3474 | We assume __X is in bytes because that's how addresses work on X86. Adding a bit offset to a byte address makes no sense. I see that is how existing Intel documentation works, which does not make it correct. To "make both in bits" means multiplying __X by 8, as in the example in my previous comment. Or coming up with a different syntax that makes the difference clear. |
LGTM.
clang/lib/Headers/avx2intrin.h | ||
---|---|---|
3474 | My intention is to match with Intrinsic Guide as possible. Multiplying by 8 cannot achive it, but I cannot deny __X in bytes makes sense. |
j