From fe617f44f3edd6aef98ce578307cf92b4b5e69af Mon Sep 17 00:00:00 2001 From: Shengchen Kan Date: Wed, 24 Jan 2024 11:01:44 +0800 Subject: [PATCH 1/3] [X86][CodeGen] Fix crash when commute operands of Instruction for code size Reported in 134fcc62786d31ab73439201dce2d73808d1785a --- llvm/lib/Target/X86/X86InstrInfo.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp index b42d8aad48b3f5..23532c44fb31e6 100644 --- a/llvm/lib/Target/X86/X86InstrInfo.cpp +++ b/llvm/lib/Target/X86/X86InstrInfo.cpp @@ -2327,29 +2327,30 @@ MachineInstr *X86InstrInfo::commuteInstructionImpl(MachineInstr &MI, bool NewMI, // If we're optimizing for size, try to use MOVSD/MOVSS. if (MI.getParent()->getParent()->getFunction().hasOptSize()) { unsigned Mask; + unsigned NewOpc; switch (Opc) { default: llvm_unreachable("Unreachable!"); case X86::BLENDPDrri: - Opc = X86::MOVSDrr; + NewOpc = X86::MOVSDrr; Mask = 0x03; break; case X86::BLENDPSrri: - Opc = X86::MOVSSrr; + NewOpc = X86::MOVSSrr; Mask = 0x0F; break; case X86::VBLENDPDrri: - Opc = X86::VMOVSDrr; + NewOpc = X86::VMOVSDrr; Mask = 0x03; break; case X86::VBLENDPSrri: - Opc = X86::VMOVSSrr; + NewOpc = X86::VMOVSSrr; Mask = 0x0F; break; } if ((MI.getOperand(3).getImm() ^ Mask) == 1) { WorkingMI = CloneIfNew(MI); - WorkingMI->setDesc(get(Opc)); + WorkingMI->setDesc(get(NewOpc)); WorkingMI->removeOperand(3); break; } From aa659f1e14d015d52c4ce38c95fd05c05f7449a9 Mon Sep 17 00:00:00 2001 From: Shengchen Kan Date: Wed, 24 Jan 2024 16:41:46 +0800 Subject: [PATCH 2/3] Add test and address review comments --- llvm/lib/Target/X86/X86InstrInfo.cpp | 38 ++++++++------------- llvm/test/CodeGen/X86/commute-blend-avx2.ll | 9 +++++ 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp index 23532c44fb31e6..26a8d0ba0cea46 100644 --- a/llvm/lib/Target/X86/X86InstrInfo.cpp +++ b/llvm/lib/Target/X86/X86InstrInfo.cpp @@ -2326,34 +2326,26 @@ MachineInstr *X86InstrInfo::commuteInstructionImpl(MachineInstr &MI, bool NewMI, case X86::VBLENDPSrri: // If we're optimizing for size, try to use MOVSD/MOVSS. if (MI.getParent()->getParent()->getFunction().hasOptSize()) { - unsigned Mask; - unsigned NewOpc; - switch (Opc) { - default: - llvm_unreachable("Unreachable!"); - case X86::BLENDPDrri: - NewOpc = X86::MOVSDrr; - Mask = 0x03; - break; - case X86::BLENDPSrri: - NewOpc = X86::MOVSSrr; - Mask = 0x0F; - break; - case X86::VBLENDPDrri: - NewOpc = X86::VMOVSDrr; - Mask = 0x03; - break; - case X86::VBLENDPSrri: - NewOpc = X86::VMOVSSrr; - Mask = 0x0F; - break; - } + unsigned Mask = (Opc == X86::BLENDPDrri || Opc == X86::VBLENDPDrri) ? 0x03: 0x0F; if ((MI.getOperand(3).getImm() ^ Mask) == 1) { +#define FROM_TO(A, B) \ + case X86::A: \ + Opc = X86::B; \ + break; + switch (Opc) { + default: + llvm_unreachable("Unreachable!"); + FROM_TO(BLENDPDrri, MOVSDrr) + FROM_TO(BLENDPSrri, MOVSSrr) + FROM_TO(VBLENDPDrri, VMOVSDrr) + FROM_TO(VBLENDPSrri, VMOVSSrr) + } WorkingMI = CloneIfNew(MI); - WorkingMI->setDesc(get(NewOpc)); + WorkingMI->setDesc(get(Opc)); WorkingMI->removeOperand(3); break; } +#undef FROM_TO } [[fallthrough]]; case X86::PBLENDWrri: diff --git a/llvm/test/CodeGen/X86/commute-blend-avx2.ll b/llvm/test/CodeGen/X86/commute-blend-avx2.ll index b5ffe78d29a610..75511104580e90 100644 --- a/llvm/test/CodeGen/X86/commute-blend-avx2.ll +++ b/llvm/test/CodeGen/X86/commute-blend-avx2.ll @@ -88,3 +88,12 @@ define <4 x double> @commute_fold_vblendpd_256(<4 x double> %a, ptr %b) #0 { ret <4 x double> %2 } declare <4 x double> @llvm.x86.avx.blend.pd.256(<4 x double>, <4 x double>, i8) nounwind readnone + +define <4 x float> @commute_vblendpd_128_for_code_size(<4 x float> %a, <4 x float> %b) optsize { +; CHECK-LABEL: commute_vblendpd_128_for_code_size: +; CHECK: # %bb.0: +; CHECK-NEXT: vblendps {{.*#+}} xmm0 = xmm1[0],xmm0[1],xmm1[2,3] +; CHECK-NEXT: retq + %r = shufflevector <4 x float> %b, <4 x float> %a, <4 x i32> + ret <4 x float> %r +} From ebc5996aba54ff30bef1c6f9af9811cb582ca98c Mon Sep 17 00:00:00 2001 From: Shengchen Kan Date: Wed, 24 Jan 2024 17:08:52 +0800 Subject: [PATCH 3/3] address review comment --- llvm/lib/Target/X86/X86InstrInfo.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp index 26a8d0ba0cea46..975d12d271d836 100644 --- a/llvm/lib/Target/X86/X86InstrInfo.cpp +++ b/llvm/lib/Target/X86/X86InstrInfo.cpp @@ -2328,9 +2328,9 @@ MachineInstr *X86InstrInfo::commuteInstructionImpl(MachineInstr &MI, bool NewMI, if (MI.getParent()->getParent()->getFunction().hasOptSize()) { unsigned Mask = (Opc == X86::BLENDPDrri || Opc == X86::VBLENDPDrri) ? 0x03: 0x0F; if ((MI.getOperand(3).getImm() ^ Mask) == 1) { -#define FROM_TO(A, B) \ - case X86::A: \ - Opc = X86::B; \ +#define FROM_TO(FROM, TO) \ + case X86::FROM: \ + Opc = X86::TO; \ break; switch (Opc) { default: