Skip to content

Commit 2ed1587

Browse files
RKSimonRKSimon
committed
[X86] Ensure asm comments only print the constant values for the vector load's register width
We were printing the entire Constant, which if we were loading from a wider constant pool entry meant that we were confusing the asm comment with upper bits that aren't actually part of the load result
1 parent ff219ea commit 2ed1587

File tree

2 files changed

+49
-27
lines changed

2 files changed

+49
-27
lines changed

‎llvm/lib/Target/X86/X86MCInstLower.cpp‎

Lines changed: 41 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1842,6 +1842,18 @@ static void addConstantComments(const MachineInstr *MI,
18421842
case X86::VMOVUPS##Suffix##rm: \
18431843
case X86::VMOVUPD##Suffix##rm:
18441844

1845+
#define CASE_128_MOV_RM() \
1846+
MOV_CASE(, ) /* SSE */ \
1847+
MOV_CASE(V, ) /* AVX-128 */ \
1848+
MOV_AVX512_CASE(Z128)
1849+
1850+
#define CASE_256_MOV_RM() \
1851+
MOV_CASE(V, Y) /* AVX-256 */ \
1852+
MOV_AVX512_CASE(Z256)
1853+
1854+
#define CASE_512_MOV_RM() \
1855+
MOV_AVX512_CASE(Z)
1856+
18451857
#define CASE_ALL_MOV_RM() \
18461858
MOV_CASE(, ) /* SSE */ \
18471859
MOV_CASE(V, ) /* AVX-128 */ \
@@ -1871,33 +1883,41 @@ static void addConstantComments(const MachineInstr *MI,
18711883
"Unexpected number of operands!");
18721884
if (auto *C = getConstantFromPool(*MI, MI->getOperand(1 + X86::AddrDisp))) {
18731885
int NumLanes = 1;
1874-
// Override NumLanes for the broadcast instructions.
1886+
int BitWidth = 128;
1887+
int CstEltSize = C->getType()->getScalarSizeInBits();
1888+
1889+
// Get destination BitWidth + override NumLanes for the broadcasts.
18751890
switch (MI->getOpcode()) {
1876-
case X86::VBROADCASTF128: NumLanes = 2; break;
1877-
case X86::VBROADCASTI128: NumLanes = 2; break;
1878-
case X86::VBROADCASTF32X4Z256rm: NumLanes = 2; break;
1879-
case X86::VBROADCASTF32X4rm: NumLanes = 4; break;
1880-
case X86::VBROADCASTF32X8rm: NumLanes = 2; break;
1881-
case X86::VBROADCASTF64X2Z128rm: NumLanes = 2; break;
1882-
case X86::VBROADCASTF64X2rm: NumLanes = 4; break;
1883-
case X86::VBROADCASTF64X4rm: NumLanes = 2; break;
1884-
case X86::VBROADCASTI32X4Z256rm: NumLanes = 2; break;
1885-
case X86::VBROADCASTI32X4rm: NumLanes = 4; break;
1886-
case X86::VBROADCASTI32X8rm: NumLanes = 2; break;
1887-
case X86::VBROADCASTI64X2Z128rm: NumLanes = 2; break;
1888-
case X86::VBROADCASTI64X2rm: NumLanes = 4; break;
1889-
case X86::VBROADCASTI64X4rm: NumLanes = 2; break;
1891+
CASE_128_MOV_RM() NumLanes = 1; BitWidth = 128; break;
1892+
CASE_256_MOV_RM() NumLanes = 1; BitWidth = 256; break;
1893+
CASE_512_MOV_RM() NumLanes = 1; BitWidth = 512; break;
1894+
case X86::VBROADCASTF128: NumLanes = 2; BitWidth = 128; break;
1895+
case X86::VBROADCASTI128: NumLanes = 2; BitWidth = 128; break;
1896+
case X86::VBROADCASTF32X4Z256rm: NumLanes = 2; BitWidth = 128; break;
1897+
case X86::VBROADCASTF32X4rm: NumLanes = 4; BitWidth = 128; break;
1898+
case X86::VBROADCASTF32X8rm: NumLanes = 2; BitWidth = 256; break;
1899+
case X86::VBROADCASTF64X2Z128rm: NumLanes = 2; BitWidth = 128; break;
1900+
case X86::VBROADCASTF64X2rm: NumLanes = 4; BitWidth = 128; break;
1901+
case X86::VBROADCASTF64X4rm: NumLanes = 2; BitWidth = 256; break;
1902+
case X86::VBROADCASTI32X4Z256rm: NumLanes = 2; BitWidth = 128; break;
1903+
case X86::VBROADCASTI32X4rm: NumLanes = 4; BitWidth = 128; break;
1904+
case X86::VBROADCASTI32X8rm: NumLanes = 2; BitWidth = 256; break;
1905+
case X86::VBROADCASTI64X2Z128rm: NumLanes = 2; BitWidth = 128; break;
1906+
case X86::VBROADCASTI64X2rm: NumLanes = 4; BitWidth = 128; break;
1907+
case X86::VBROADCASTI64X4rm: NumLanes = 2; BitWidth = 256; break;
18901908
}
18911909

18921910
std::string Comment;
18931911
raw_string_ostream CS(Comment);
18941912
const MachineOperand &DstOp = MI->getOperand(0);
18951913
CS << X86ATTInstPrinter::getRegisterName(DstOp.getReg()) << " = ";
18961914
if (auto *CDS = dyn_cast<ConstantDataSequential>(C)) {
1915+
int NumElements = CDS->getNumElements();
1916+
if ((BitWidth % CstEltSize) == 0)
1917+
NumElements = std::min<int>(NumElements, BitWidth / CstEltSize);
18971918
CS << "[";
18981919
for (int l = 0; l != NumLanes; ++l) {
1899-
for (int i = 0, NumElements = CDS->getNumElements(); i < NumElements;
1900-
++i) {
1920+
for (int i = 0; i < NumElements; ++i) {
19011921
if (i != 0 || l != 0)
19021922
CS << ",";
19031923
if (CDS->getElementType()->isIntegerTy())
@@ -1913,10 +1933,12 @@ static void addConstantComments(const MachineInstr *MI,
19131933
CS << "]";
19141934
OutStreamer.AddComment(CS.str());
19151935
} else if (auto *CV = dyn_cast<ConstantVector>(C)) {
1936+
int NumOperands = CV->getNumOperands();
1937+
if ((BitWidth % CstEltSize) == 0)
1938+
NumOperands = std::min<int>(NumOperands, BitWidth / CstEltSize);
19161939
CS << "<";
19171940
for (int l = 0; l != NumLanes; ++l) {
1918-
for (int i = 0, NumOperands = CV->getNumOperands(); i < NumOperands;
1919-
++i) {
1941+
for (int i = 0; i < NumOperands; ++i) {
19201942
if (i != 0 || l != 0)
19211943
CS << ",";
19221944
printConstant(CV->getOperand(i),

‎llvm/test/CodeGen/X86/insert-into-constant-vector.ll‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ define <8 x i64> @elt5_v8i64(i64 %x) {
412412
;
413413
; X64-AVX1-LABEL: elt5_v8i64:
414414
; X64-AVX1: # %bb.0:
415-
; X64-AVX1-NEXT: vmovdqa {{.*#+}} xmm0 = <4,u,6,7>
415+
; X64-AVX1-NEXT: vmovdqa {{.*#+}} xmm0 = <4,u>
416416
; X64-AVX1-NEXT: vpinsrq $1, %rdi, %xmm0, %xmm0
417417
; X64-AVX1-NEXT: vblendps {{.*#+}} ymm1 = ymm0[0,1,2,3],mem[4,5,6,7]
418418
; X64-AVX1-NEXT: vmovaps {{.*#+}} ymm0 = [42,1,2,3]
@@ -429,7 +429,7 @@ define <8 x i64> @elt5_v8i64(i64 %x) {
429429
;
430430
; X64-AVX2-LABEL: elt5_v8i64:
431431
; X64-AVX2: # %bb.0:
432-
; X64-AVX2-NEXT: vmovdqa {{.*#+}} xmm0 = <4,u,6,7>
432+
; X64-AVX2-NEXT: vmovdqa {{.*#+}} xmm0 = <4,u>
433433
; X64-AVX2-NEXT: vpinsrq $1, %rdi, %xmm0, %xmm0
434434
; X64-AVX2-NEXT: vpblendd {{.*#+}} ymm1 = ymm0[0,1,2,3],mem[4,5,6,7]
435435
; X64-AVX2-NEXT: vmovaps {{.*#+}} ymm0 = [42,1,2,3]
@@ -478,47 +478,47 @@ define <8 x double> @elt1_v8f64(double %x) {
478478
;
479479
; X86-AVX1-LABEL: elt1_v8f64:
480480
; X86-AVX1: # %bb.0:
481-
; X86-AVX1-NEXT: vmovaps {{.*#+}} xmm0 = <4.2E+1,u,2.0E+0,3.0E+0>
481+
; X86-AVX1-NEXT: vmovaps {{.*#+}} xmm0 = <4.2E+1,u>
482482
; X86-AVX1-NEXT: vmovhps {{.*#+}} xmm0 = xmm0[0,1],mem[0,1]
483483
; X86-AVX1-NEXT: vblendps {{.*#+}} ymm0 = ymm0[0,1,2,3],mem[4,5,6,7]
484484
; X86-AVX1-NEXT: vmovaps {{.*#+}} ymm1 = [4.0E+0,5.0E+0,6.0E+0,7.0E+0]
485485
; X86-AVX1-NEXT: retl
486486
;
487487
; X64-AVX1-LABEL: elt1_v8f64:
488488
; X64-AVX1: # %bb.0:
489-
; X64-AVX1-NEXT: vmovaps {{.*#+}} xmm1 = <4.2E+1,u,2.0E+0,3.0E+0>
489+
; X64-AVX1-NEXT: vmovaps {{.*#+}} xmm1 = <4.2E+1,u>
490490
; X64-AVX1-NEXT: vmovlhps {{.*#+}} xmm0 = xmm1[0],xmm0[0]
491491
; X64-AVX1-NEXT: vblendps {{.*#+}} ymm0 = ymm0[0,1,2,3],mem[4,5,6,7]
492492
; X64-AVX1-NEXT: vmovaps {{.*#+}} ymm1 = [4.0E+0,5.0E+0,6.0E+0,7.0E+0]
493493
; X64-AVX1-NEXT: retq
494494
;
495495
; X86-AVX2-LABEL: elt1_v8f64:
496496
; X86-AVX2: # %bb.0:
497-
; X86-AVX2-NEXT: vmovaps {{.*#+}} xmm0 = <4.2E+1,u,2.0E+0,3.0E+0>
497+
; X86-AVX2-NEXT: vmovaps {{.*#+}} xmm0 = <4.2E+1,u>
498498
; X86-AVX2-NEXT: vmovhps {{.*#+}} xmm0 = xmm0[0,1],mem[0,1]
499499
; X86-AVX2-NEXT: vblendps {{.*#+}} ymm0 = ymm0[0,1,2,3],mem[4,5,6,7]
500500
; X86-AVX2-NEXT: vmovaps {{.*#+}} ymm1 = [4.0E+0,5.0E+0,6.0E+0,7.0E+0]
501501
; X86-AVX2-NEXT: retl
502502
;
503503
; X64-AVX2-LABEL: elt1_v8f64:
504504
; X64-AVX2: # %bb.0:
505-
; X64-AVX2-NEXT: vmovaps {{.*#+}} xmm1 = <4.2E+1,u,2.0E+0,3.0E+0>
505+
; X64-AVX2-NEXT: vmovaps {{.*#+}} xmm1 = <4.2E+1,u>
506506
; X64-AVX2-NEXT: vmovlhps {{.*#+}} xmm0 = xmm1[0],xmm0[0]
507507
; X64-AVX2-NEXT: vblendps {{.*#+}} ymm0 = ymm0[0,1,2,3],mem[4,5,6,7]
508508
; X64-AVX2-NEXT: vmovaps {{.*#+}} ymm1 = [4.0E+0,5.0E+0,6.0E+0,7.0E+0]
509509
; X64-AVX2-NEXT: retq
510510
;
511511
; X86-AVX512F-LABEL: elt1_v8f64:
512512
; X86-AVX512F: # %bb.0:
513-
; X86-AVX512F-NEXT: vmovaps {{.*#+}} xmm0 = <4.2E+1,u,2.0E+0,3.0E+0,4.0E+0,5.0E+0,6.0E+0,7.0E+0>
513+
; X86-AVX512F-NEXT: vmovaps {{.*#+}} xmm0 = <4.2E+1,u>
514514
; X86-AVX512F-NEXT: vmovhps {{.*#+}} xmm0 = xmm0[0,1],mem[0,1]
515515
; X86-AVX512F-NEXT: vmovaps {{.*#+}} zmm1 = <4.2E+1,u,2.0E+0,3.0E+0,4.0E+0,5.0E+0,6.0E+0,7.0E+0>
516516
; X86-AVX512F-NEXT: vinsertf32x4 $0, %xmm0, %zmm1, %zmm0
517517
; X86-AVX512F-NEXT: retl
518518
;
519519
; X64-AVX512F-LABEL: elt1_v8f64:
520520
; X64-AVX512F: # %bb.0:
521-
; X64-AVX512F-NEXT: vmovaps {{.*#+}} xmm1 = <4.2E+1,u,2.0E+0,3.0E+0,4.0E+0,5.0E+0,6.0E+0,7.0E+0>
521+
; X64-AVX512F-NEXT: vmovaps {{.*#+}} xmm1 = <4.2E+1,u>
522522
; X64-AVX512F-NEXT: vmovlhps {{.*#+}} xmm0 = xmm1[0],xmm0[0]
523523
; X64-AVX512F-NEXT: vmovaps {{.*#+}} zmm1 = <4.2E+1,u,2.0E+0,3.0E+0,4.0E+0,5.0E+0,6.0E+0,7.0E+0>
524524
; X64-AVX512F-NEXT: vinsertf32x4 $0, %xmm0, %zmm1, %zmm0

0 commit comments

Comments
 (0)