probinson added inline comments.
================ Comment at: clang/lib/Headers/avx2intrin.h:3474 +/// IF __M[j+31] == 1 +/// result[j+31:j] := Load32(__X+(i*4)) +/// ELSE ---------------- pengfei wrote: > probinson wrote: > > pengfei wrote: > > > A more intrinsic guide format is `MEM[__X+j:j]` > > LoadXX is the syntax in the gather intrinsics, e.g. _mm_mask_i32gather_pd. > > I'd prefer to be consistent. > I think the problem here is the measurement is easily confusing. > From C point of view, `__X` is a `int` pointer, so we should `+ i` rather > than `i * 4` > From the other part of the code, we are measuring in bits, but here `i * 4` > is a byte offset. 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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153993/new/ https://reviews.llvm.org/D153993 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
