[llvm] b215a26 - [AMDGPU] Update LiveVariables in convertToThreeAddress()
Ruiling Song via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 12 17:12:54 PDT 2020
Author: Ruiling Song
Date: 2020-10-13T08:12:20+08:00
New Revision: b215a26628feae349d663f687efe475d622970b7
URL: https://github.com/llvm/llvm-project/commit/b215a26628feae349d663f687efe475d622970b7
DIFF: https://github.com/llvm/llvm-project/commit/b215a26628feae349d663f687efe475d622970b7.diff
LOG: [AMDGPU] Update LiveVariables in convertToThreeAddress()
This can fix an asan failure like below.
==15856==ERROR: AddressSanitizer: use-after-poison on address ...
READ of size 8 at 0x6210001a3cb0 thread T0
#0 llvm::MachineInstr::getParent()
#1 llvm::LiveVariables::VarInfo::findKill()
#2 TwoAddressInstructionPass::rescheduleMIBelowKill()
#3 TwoAddressInstructionPass::tryInstructionTransform()
#4 TwoAddressInstructionPass::runOnMachineFunction()
We need to update the Kills if we replace instructions. The Kills
may be later accessed within TwoAddressInstruction pass.
Differential Revision: https://reviews.llvm.org/D89092
Added:
llvm/test/CodeGen/AMDGPU/stale-livevar-in-twoaddr-pass.mir
Modified:
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index a2cbe2735220..c72e9ede080e 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -15,10 +15,10 @@
#include "AMDGPU.h"
#include "AMDGPUSubtarget.h"
#include "GCNHazardRecognizer.h"
+#include "MCTargetDesc/AMDGPUMCTargetDesc.h"
#include "SIDefines.h"
#include "SIMachineFunctionInfo.h"
#include "SIRegisterInfo.h"
-#include "MCTargetDesc/AMDGPUMCTargetDesc.h"
#include "Utils/AMDGPUBaseInfo.h"
#include "llvm/ADT/APInt.h"
#include "llvm/ADT/ArrayRef.h"
@@ -28,6 +28,7 @@
#include "llvm/Analysis/AliasAnalysis.h"
#include "llvm/Analysis/MemoryLocation.h"
#include "llvm/Analysis/ValueTracking.h"
+#include "llvm/CodeGen/LiveVariables.h"
#include "llvm/CodeGen/MachineBasicBlock.h"
#include "llvm/CodeGen/MachineDominators.h"
#include "llvm/CodeGen/MachineFrameInfo.h"
@@ -2841,6 +2842,18 @@ static int64_t getFoldableImm(const MachineOperand* MO) {
return AMDGPU::NoRegister;
}
+static void updateLiveVariables(LiveVariables *LV, MachineInstr &MI,
+ MachineInstr &NewMI) {
+ if (LV) {
+ unsigned NumOps = MI.getNumOperands();
+ for (unsigned I = 1; I < NumOps; ++I) {
+ MachineOperand &Op = MI.getOperand(I);
+ if (Op.isReg() && Op.isKill())
+ LV->replaceKillInstruction(Op.getReg(), MI, NewMI);
+ }
+ }
+}
+
MachineInstr *SIInstrInfo::convertToThreeAddress(MachineFunction::iterator &MBB,
MachineInstr &MI,
LiveVariables *LV) const {
@@ -2888,43 +2901,53 @@ MachineInstr *SIInstrInfo::convertToThreeAddress(MachineFunction::iterator &MBB,
const MachineOperand *Src2 = getNamedOperand(MI, AMDGPU::OpName::src2);
const MachineOperand *Clamp = getNamedOperand(MI, AMDGPU::OpName::clamp);
const MachineOperand *Omod = getNamedOperand(MI, AMDGPU::OpName::omod);
+ MachineInstrBuilder MIB;
if (!Src0Mods && !Src1Mods && !Clamp && !Omod &&
// If we have an SGPR input, we will violate the constant bus restriction.
- (ST.getConstantBusLimit(Opc) > 1 ||
- !Src0->isReg() ||
+ (ST.getConstantBusLimit(Opc) > 1 || !Src0->isReg() ||
!RI.isSGPRReg(MBB->getParent()->getRegInfo(), Src0->getReg()))) {
if (auto Imm = getFoldableImm(Src2)) {
unsigned NewOpc =
- IsFMA ? (IsF16 ? AMDGPU::V_FMAAK_F16 : AMDGPU::V_FMAAK_F32)
- : (IsF16 ? AMDGPU::V_MADAK_F16 : AMDGPU::V_MADAK_F32);
- if (pseudoToMCOpcode(NewOpc) != -1)
- return BuildMI(*MBB, MI, MI.getDebugLoc(), get(NewOpc))
- .add(*Dst)
- .add(*Src0)
- .add(*Src1)
- .addImm(Imm);
- }
- unsigned NewOpc =
- IsFMA ? (IsF16 ? AMDGPU::V_FMAMK_F16 : AMDGPU::V_FMAMK_F32)
- : (IsF16 ? AMDGPU::V_MADMK_F16 : AMDGPU::V_MADMK_F32);
+ IsFMA ? (IsF16 ? AMDGPU::V_FMAAK_F16 : AMDGPU::V_FMAAK_F32)
+ : (IsF16 ? AMDGPU::V_MADAK_F16 : AMDGPU::V_MADAK_F32);
+ if (pseudoToMCOpcode(NewOpc) != -1) {
+ MIB = BuildMI(*MBB, MI, MI.getDebugLoc(), get(NewOpc))
+ .add(*Dst)
+ .add(*Src0)
+ .add(*Src1)
+ .addImm(Imm);
+ updateLiveVariables(LV, MI, *MIB);
+ return MIB;
+ }
+ }
+ unsigned NewOpc = IsFMA
+ ? (IsF16 ? AMDGPU::V_FMAMK_F16 : AMDGPU::V_FMAMK_F32)
+ : (IsF16 ? AMDGPU::V_MADMK_F16 : AMDGPU::V_MADMK_F32);
if (auto Imm = getFoldableImm(Src1)) {
- if (pseudoToMCOpcode(NewOpc) != -1)
- return BuildMI(*MBB, MI, MI.getDebugLoc(), get(NewOpc))
- .add(*Dst)
- .add(*Src0)
- .addImm(Imm)
- .add(*Src2);
+ if (pseudoToMCOpcode(NewOpc) != -1) {
+ MIB = BuildMI(*MBB, MI, MI.getDebugLoc(), get(NewOpc))
+ .add(*Dst)
+ .add(*Src0)
+ .addImm(Imm)
+ .add(*Src2);
+ updateLiveVariables(LV, MI, *MIB);
+ return MIB;
+ }
}
if (auto Imm = getFoldableImm(Src0)) {
if (pseudoToMCOpcode(NewOpc) != -1 &&
- isOperandLegal(MI, AMDGPU::getNamedOperandIdx(NewOpc,
- AMDGPU::OpName::src0), Src1))
- return BuildMI(*MBB, MI, MI.getDebugLoc(), get(NewOpc))
- .add(*Dst)
- .add(*Src1)
- .addImm(Imm)
- .add(*Src2);
+ isOperandLegal(
+ MI, AMDGPU::getNamedOperandIdx(NewOpc, AMDGPU::OpName::src0),
+ Src1)) {
+ MIB = BuildMI(*MBB, MI, MI.getDebugLoc(), get(NewOpc))
+ .add(*Dst)
+ .add(*Src1)
+ .addImm(Imm)
+ .add(*Src2);
+ updateLiveVariables(LV, MI, *MIB);
+ return MIB;
+ }
}
}
@@ -2933,16 +2956,18 @@ MachineInstr *SIInstrInfo::convertToThreeAddress(MachineFunction::iterator &MBB,
if (pseudoToMCOpcode(NewOpc) == -1)
return nullptr;
- return BuildMI(*MBB, MI, MI.getDebugLoc(), get(NewOpc))
- .add(*Dst)
- .addImm(Src0Mods ? Src0Mods->getImm() : 0)
- .add(*Src0)
- .addImm(Src1Mods ? Src1Mods->getImm() : 0)
- .add(*Src1)
- .addImm(0) // Src mods
- .add(*Src2)
- .addImm(Clamp ? Clamp->getImm() : 0)
- .addImm(Omod ? Omod->getImm() : 0);
+ MIB = BuildMI(*MBB, MI, MI.getDebugLoc(), get(NewOpc))
+ .add(*Dst)
+ .addImm(Src0Mods ? Src0Mods->getImm() : 0)
+ .add(*Src0)
+ .addImm(Src1Mods ? Src1Mods->getImm() : 0)
+ .add(*Src1)
+ .addImm(0) // Src mods
+ .add(*Src2)
+ .addImm(Clamp ? Clamp->getImm() : 0)
+ .addImm(Omod ? Omod->getImm() : 0);
+ updateLiveVariables(LV, MI, *MIB);
+ return MIB;
}
// It's not generally safe to move VALU instructions across these since it will
diff --git a/llvm/test/CodeGen/AMDGPU/stale-livevar-in-twoaddr-pass.mir b/llvm/test/CodeGen/AMDGPU/stale-livevar-in-twoaddr-pass.mir
new file mode 100644
index 000000000000..264932ae9f43
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/stale-livevar-in-twoaddr-pass.mir
@@ -0,0 +1,36 @@
+# RUN: llc -march=amdgcn -mcpu=gfx900 -run-pass=livevars,phi-node-elimination,twoaddressinstruction -verify-machineinstrs -o - %s | FileCheck %s
+# This used to fail under ASAN enabled build because we didn't update LiveVariables in SIInstrInfo::convertToThreeAddress()
+# CHECK: _amdgpu_ps_main
+
+---
+name: _amdgpu_ps_main
+alignment: 1
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $sgpr2, $vgpr2, $vgpr3
+
+ %0:vgpr_32 = COPY $vgpr3
+ %1:vgpr_32 = COPY $vgpr2
+ S_BRANCH %bb.3
+
+ bb.1:
+ %2:vgpr_32 = V_MAC_F32_e32 0, %0, %1, implicit $mode, implicit $exec
+ %3:vgpr_32 = V_MED3_F32 0, %1, 0, %2, 0, %2, 0, 0, implicit $mode, implicit $exec
+
+ bb.2:
+ %4:vgpr_32 = PHI %5, %bb.3, %3, %bb.1
+ SI_END_CF %6, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ EXP_DONE 0, %4, %4, %4, %4, -1, 0, 15, implicit $exec
+ S_ENDPGM 0
+
+ bb.3:
+ successors: %bb.1, %bb.2
+
+ %5:vgpr_32 = V_MAC_F32_e32 0, %1, %0, implicit $mode, implicit $exec
+ %7:vgpr_32 = V_CVT_I32_F32_e32 %5, implicit $mode, implicit $exec
+ %8:sreg_64 = V_CMP_NE_U32_e64 1, %7, implicit $exec
+ %6:sreg_64 = SI_IF %8, %bb.2, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ S_BRANCH %bb.1
+
+...
More information about the llvm-commits
mailing list