Skip to content

Commit 457e548

Browse files
chaitanyavchaitanyav
committed
Address review feedback
- Rename ByteInQWord to ByteIdx for loop counter within qword - Add SelIdx for absolute byte index used in mask selection - Use APInt::insertBits() instead of manual shift/OR for building SourceQWord - Remove unnecessary const from local variables - Move constexpr after __attribute__ in header macros - Minor formatting cleanup
1 parent 71dbbff commit 457e548

File tree

4 files changed

+31
-39
lines changed

4 files changed

+31
-39
lines changed

‎clang/lib/AST/ByteCode/InterpBuiltin.cpp‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3495,36 +3495,33 @@ static bool interp__builtin_ia32_shufbitqmb_mask(InterpState &S, CodePtr OpPC,
34953495
PrimType ShuffleMaskElemT =
34963496
*S.getContext().classify(ShuffleMaskVecT->getElementType());
34973497

3498-
const unsigned NumBytesInQWord = 8;
3499-
const unsigned NumBitsInByte = 8;
3500-
const unsigned NumBytes = SourceVecT->getNumElements();
3501-
const unsigned NumQWords = NumBytes / NumBytesInQWord;
3502-
const unsigned RetWidth = ZeroMask.getBitWidth();
3498+
unsigned NumBytesInQWord = 8;
3499+
unsigned NumBitsInByte = 8;
3500+
unsigned NumBytes = SourceVecT->getNumElements();
3501+
unsigned NumQWords = NumBytes / NumBytesInQWord;
3502+
unsigned RetWidth = ZeroMask.getBitWidth();
35033503
APSInt RetMask(llvm::APInt(RetWidth, 0), /*isUnsigned=*/true);
35043504

35053505
for (unsigned QWordId = 0; QWordId != NumQWords; ++QWordId) {
3506-
35073506
APInt SourceQWord(64, 0);
3508-
for (unsigned ByteInQWord = 0; ByteInQWord != NumBytesInQWord;
3509-
++ByteInQWord) {
3507+
for (unsigned ByteIdx = 0; ByteIdx != NumBytesInQWord; ++ByteIdx) {
35103508
uint64_t Byte = 0;
35113509
INT_TYPE_SWITCH(SourceElemT, {
35123510
Byte = static_cast<uint64_t>(
3513-
Source.elem<T>(QWordId * NumBytesInQWord + ByteInQWord));
3511+
Source.elem<T>(QWordId * NumBytesInQWord + ByteIdx));
35143512
});
3515-
SourceQWord |= (Byte & 0xFF) << (ByteInQWord * NumBitsInByte);
3513+
SourceQWord.insertBits(APInt(8, Byte & 0xFF), ByteIdx * NumBitsInByte);
35163514
}
35173515

3518-
for (unsigned ByteInQWord = 0; ByteInQWord != NumBytesInQWord;
3519-
++ByteInQWord) {
3520-
unsigned ByteIdx = QWordId * NumBytesInQWord + ByteInQWord;
3516+
for (unsigned ByteIdx = 0; ByteIdx != NumBytesInQWord; ++ByteIdx) {
3517+
unsigned SelIdx = QWordId * NumBytesInQWord + ByteIdx;
35213518
unsigned M = 0;
35223519
INT_TYPE_SWITCH(ShuffleMaskElemT, {
3523-
M = static_cast<unsigned>(ShuffleMask.elem<T>(ByteIdx)) & 0x3F;
3520+
M = static_cast<unsigned>(ShuffleMask.elem<T>(SelIdx)) & 0x3F;
35243521
});
35253522

3526-
if (ZeroMask[ByteIdx]) {
3527-
RetMask.setBitVal(ByteIdx, SourceQWord[M]);
3523+
if (ZeroMask[SelIdx]) {
3524+
RetMask.setBitVal(SelIdx, SourceQWord[M]);
35283525
}
35293526
}
35303527
}

‎clang/lib/AST/ExprConstant.cpp‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16729,24 +16729,20 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
1672916729
APSInt RetMask(llvm::APInt(RetWidth, 0), /*isUnsigned=*/true);
1673016730

1673116731
for (unsigned QWordId = 0; QWordId != NumQWords; ++QWordId) {
16732-
1673316732
APInt SourceQWord(64, 0);
16734-
for (unsigned ByteInQWord = 0; ByteInQWord != NumBytesInQWord;
16735-
++ByteInQWord) {
16736-
uint64_t Byte =
16737-
Source.getVectorElt(QWordId * NumBytesInQWord + ByteInQWord)
16738-
.getInt()
16739-
.getZExtValue();
16740-
SourceQWord |= (Byte & 0xFF) << (ByteInQWord * NumBitsInByte);
16733+
for (unsigned ByteIdx = 0; ByteIdx != NumBytesInQWord; ++ByteIdx) {
16734+
uint64_t Byte = Source.getVectorElt(QWordId * NumBytesInQWord + ByteIdx)
16735+
.getInt()
16736+
.getZExtValue();
16737+
SourceQWord.insertBits(APInt(8, Byte & 0xFF), ByteIdx * NumBitsInByte);
1674116738
}
1674216739

16743-
for (unsigned ByteInQWord = 0; ByteInQWord != NumBytesInQWord;
16744-
++ByteInQWord) {
16745-
unsigned ByteIdx = QWordId * NumBytesInQWord + ByteInQWord;
16740+
for (unsigned ByteIdx = 0; ByteIdx != NumBytesInQWord; ++ByteIdx) {
16741+
unsigned SelIdx = QWordId * NumBytesInQWord + ByteIdx;
1674616742
unsigned M =
16747-
ShuffleMask.getVectorElt(ByteIdx).getInt().getZExtValue() & 0x3F;
16748-
if (ZeroMask[ByteIdx]) {
16749-
RetMask.setBitVal(ByteIdx, SourceQWord[M]);
16743+
ShuffleMask.getVectorElt(SelIdx).getInt().getZExtValue() & 0x3F;
16744+
if (ZeroMask[SelIdx]) {
16745+
RetMask.setBitVal(SelIdx, SourceQWord[M]);
1675016746
}
1675116747
}
1675216748
}

‎clang/lib/Headers/avx512bitalgintrin.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,8 @@
1717
/* Define the default attributes for the functions in this file. */
1818
#if defined(__cplusplus) && (__cplusplus >= 201103L)
1919
#define __DEFAULT_FN_ATTRS \
20-
constexpr \
21-
__attribute__((__always_inline__, __nodebug__, \
22-
__target__("avx512bitalg"), __min_vector_width__(512)))
20+
__attribute__((__always_inline__, __nodebug__, __target__("avx512bitalg"), \
21+
__min_vector_width__(512))) constexpr
2322
#else
2423
#define __DEFAULT_FN_ATTRS \
2524
__attribute__((__always_inline__, __nodebug__, __target__("avx512bitalg"), \

‎clang/lib/Headers/avx512vlbitalgintrin.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@
1717
/* Define the default attributes for the functions in this file. */
1818
#if defined(__cplusplus) && (__cplusplus >= 201103L)
1919
#define __DEFAULT_FN_ATTRS128 \
20-
constexpr __attribute__((__always_inline__, __nodebug__, \
21-
__target__("avx512vl,avx512bitalg"), \
22-
__min_vector_width__(128)))
20+
__attribute__((__always_inline__, __nodebug__, \
21+
__target__("avx512vl,avx512bitalg"), \
22+
__min_vector_width__(128))) constexpr
2323
#define __DEFAULT_FN_ATTRS256 \
24-
constexpr __attribute__((__always_inline__, __nodebug__, \
25-
__target__("avx512vl,avx512bitalg"), \
26-
__min_vector_width__(256)))
24+
__attribute__((__always_inline__, __nodebug__, \
25+
__target__("avx512vl,avx512bitalg"), \
26+
__min_vector_width__(256))) constexpr
2727
#else
2828
#define __DEFAULT_FN_ATTRS128 \
2929
__attribute__((__always_inline__, __nodebug__, \

0 commit comments

Comments
 (0)