<div dir="ltr">Test coverage?<br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 2, 2016 at 7:53 PM, Tom Stellard via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: tstellar<br>
Date: Wed Mar 2 21:53:29 2016<br>
New Revision: 262579<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=262579&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=262579&view=rev</a><br>
Log:<br>
AMDGPU: Insert two S_NOP instructions for every high level source statement.<br>
<br>
Patch by: Konstantin Zhuravlyov<br>
<br>
Summary: Tools, such as debugger, need to pause execution based on user input (i.e. breakpoint). In order to do this, two S_NOP instructions are inserted for each high level source statement: one before first isa instruction of high level source statement, and one after last isa instruction of high level source statement. Further, debugger may replace S_NOP instructions with S_TRAP instructions based on user input.<br></blockquote><div><br></div><div>What behavior do you intend if there are multiple separate instances of the same line:<br><br>A<br>B<br>A<br>C<br><br>As it stands you'll get a NOP before the first A and a NOP after the last A (for A) but not at the end of the first A or the start of the second?<br><br>Also, it seems like you'll end up with two NOPs for every transition (modulo the above oddity) - eg: A, B, C -> NOP, A, NOP, NOP, B, NOP, NOP, C<br><br>Is that intentional? To what end? & why's this scheme necessary compared to debuggers on existing platforms that don't need all these NOPs to be able to replace with traps, etc?<br><br>(& why the leading NOP, but not the trailing NOP?)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Reviewers: tstellarAMD, arsenm<br>
<br>
Subscribers: echristo, dblaikie, arsenm, llvm-commits<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D17454" rel="noreferrer" target="_blank">http://reviews.llvm.org/D17454</a><br>
<br>
Added:<br>
llvm/trunk/lib/Target/AMDGPU/SIInsertNopsPass.cpp<br>
Modified:<br>
llvm/trunk/lib/Target/AMDGPU/AMDGPU.h<br>
llvm/trunk/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp<br>
llvm/trunk/lib/Target/AMDGPU/CMakeLists.txt<br>
<br>
Modified: llvm/trunk/lib/Target/AMDGPU/AMDGPU.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/AMDGPU.h?rev=262579&r1=262578&r2=262579&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/AMDGPU.h?rev=262579&r1=262578&r2=262579&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/AMDGPU/AMDGPU.h (original)<br>
+++ llvm/trunk/lib/Target/AMDGPU/AMDGPU.h Wed Mar 2 21:53:29 2016<br>
@@ -49,6 +49,7 @@ FunctionPass *createSIFixControlFlowLive<br>
FunctionPass *createSIFixSGPRCopiesPass();<br>
FunctionPass *createSIFixSGPRLiveRangesPass();<br>
FunctionPass *createSICodeEmitterPass(formatted_raw_ostream &OS);<br>
+FunctionPass *createSIInsertNopsPass();<br>
FunctionPass *createSIInsertWaitsPass();<br>
<br>
ScheduleDAGInstrs *createSIMachineScheduler(MachineSchedContext *C);<br>
@@ -97,6 +98,9 @@ extern char &AMDGPUAnnotateUniformValues<br>
void initializeSIAnnotateControlFlowPass(PassRegistry&);<br>
extern char &SIAnnotateControlFlowPassID;<br>
<br>
+void initializeSIInsertNopsPass(PassRegistry&);<br>
+extern char &SIInsertNopsID;<br>
+<br>
void initializeSIInsertWaitsPass(PassRegistry&);<br>
extern char &SIInsertWaitsID;<br>
<br>
<br>
Modified: llvm/trunk/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp?rev=262579&r1=262578&r2=262579&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp?rev=262579&r1=262578&r2=262579&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp (original)<br>
+++ llvm/trunk/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp Wed Mar 2 21:53:29 2016<br>
@@ -30,6 +30,7 @@<br>
#include "llvm/IR/Verifier.h"<br>
#include "llvm/MC/MCAsmInfo.h"<br>
#include "llvm/IR/LegacyPassManager.h"<br>
+#include "llvm/Support/CommandLine.h"<br>
#include "llvm/Support/TargetRegistry.h"<br>
#include "llvm/Support/raw_os_ostream.h"<br>
#include "llvm/Transforms/IPO.h"<br>
@@ -54,6 +55,7 @@ extern "C" void LLVMInitializeAMDGPUTarg<br>
initializeAMDGPUAnnotateUniformValuesPass(*PR);<br>
initializeAMDGPUPromoteAllocaPass(*PR);<br>
initializeSIAnnotateControlFlowPass(*PR);<br>
+ initializeSIInsertNopsPass(*PR);<br>
initializeSIInsertWaitsPass(*PR);<br>
initializeSILowerControlFlowPass(*PR);<br>
}<br>
@@ -145,6 +147,12 @@ GCNTargetMachine::GCNTargetMachine(const<br>
//===----------------------------------------------------------------------===//<br>
<br>
namespace {<br>
+<br>
+cl::opt<bool> InsertNops(<br>
+ "amdgpu-insert-nops",<br>
+ cl::desc("Insert two nop instructions for each high level source statement"),<br>
+ cl::init(false));<br>
+<br>
class AMDGPUPassConfig : public TargetPassConfig {<br>
public:<br>
AMDGPUPassConfig(TargetMachine *TM, PassManagerBase &PM)<br>
@@ -364,6 +372,9 @@ void GCNPassConfig::addPreSched2() {<br>
void GCNPassConfig::addPreEmitPass() {<br>
addPass(createSIInsertWaitsPass(), false);<br>
addPass(createSILowerControlFlowPass(), false);<br>
+ if (InsertNops) {<br>
+ addPass(createSIInsertNopsPass(), false);<br>
+ }<br></blockquote><div><br></div><div>We usually skip {} on single-line blocks ^ (though I don't think it's a style guide rule).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
}<br>
<br>
TargetPassConfig *GCNTargetMachine::createPassConfig(PassManagerBase &PM) {<br>
<br>
Modified: llvm/trunk/lib/Target/AMDGPU/CMakeLists.txt<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/CMakeLists.txt?rev=262579&r1=262578&r2=262579&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/CMakeLists.txt?rev=262579&r1=262578&r2=262579&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/AMDGPU/CMakeLists.txt (original)<br>
+++ llvm/trunk/lib/Target/AMDGPU/CMakeLists.txt Wed Mar 2 21:53:29 2016<br>
@@ -51,6 +51,7 @@ add_llvm_target(AMDGPUCodeGen<br>
SIFixSGPRLiveRanges.cpp<br>
SIFoldOperands.cpp<br>
SIFrameLowering.cpp<br>
+ SIInsertNopsPass.cpp<br>
SIInsertWaits.cpp<br>
SIInstrInfo.cpp<br>
SIISelLowering.cpp<br>
<br>
Added: llvm/trunk/lib/Target/AMDGPU/SIInsertNopsPass.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/SIInsertNopsPass.cpp?rev=262579&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/SIInsertNopsPass.cpp?rev=262579&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/AMDGPU/SIInsertNopsPass.cpp (added)<br>
+++ llvm/trunk/lib/Target/AMDGPU/SIInsertNopsPass.cpp Wed Mar 2 21:53:29 2016<br>
@@ -0,0 +1,94 @@<br>
+//===--- SIInsertNopsPass.cpp - Use predicates for control flow -----------===//<br>
+//<br>
+// The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+//<br>
+/// \file<br>
+/// \brief Insert two S_NOP instructions for every high level source statement.<br>
+///<br>
+/// Tools, such as debugger, need to pause execution based on user input (i.e.<br>
+/// breakpoint). In order to do this, two S_NOP instructions are inserted for<br>
+/// each high level source statement: one before first isa instruction of high<br>
+/// level source statement, and one after last isa instruction of high level<br>
+/// source statement. Further, debugger may replace S_NOP instructions with<br>
+/// S_TRAP instructions based on user input.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+#include "SIInstrInfo.h"<br>
+#include "llvm/ADT/DenseMap.h"<br>
+#include "llvm/CodeGen/MachineFunction.h"<br>
+#include "llvm/CodeGen/MachineFunctionPass.h"<br>
+#include "llvm/CodeGen/MachineInstrBuilder.h"<br>
+using namespace llvm;<br>
+<br>
+#define DEBUG_TYPE "si-insert-nops"<br>
+#define PASS_NAME "SI Insert Nops"<br>
+<br>
+namespace {<br>
+<br>
+class SIInsertNops : public MachineFunctionPass {<br>
+public:<br>
+ static char ID;<br>
+<br>
+ SIInsertNops() : MachineFunctionPass(ID) { }<br>
+ const char *getPassName() const override { return PASS_NAME; }<br>
+<br>
+ bool runOnMachineFunction(MachineFunction &MF) override;<br>
+};<br>
+<br>
+} // anonymous namespace<br>
+<br>
+INITIALIZE_PASS(SIInsertNops, DEBUG_TYPE, PASS_NAME, false, false)<br>
+<br>
+char SIInsertNops::ID = 0;<br>
+char &llvm::SIInsertNopsID = SIInsertNops::ID;<br>
+<br>
+FunctionPass *llvm::createSIInsertNopsPass() {<br>
+ return new SIInsertNops();<br>
+}<br>
+<br>
+bool SIInsertNops::runOnMachineFunction(MachineFunction &MF) {<br>
+ const SIInstrInfo *TII =<br>
+ static_cast<const SIInstrInfo*>(MF.getSubtarget().getInstrInfo());<br>
+<br>
+ DenseMap<unsigned, MachineBasicBlock::iterator> LineToInst;<br>
+ for (auto MBB = MF.begin(); MBB != MF.end(); ++MBB) {<br></blockquote><div><br>Range-for ^ ?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ for (auto MI = MBB->begin(); MI != MBB->end(); ++MI) {<br>
+ if (MI->isDebugValue() || !MI->getDebugLoc()) {<br>
+ continue;<br>
+ }<br>
+ auto DL = MI->getDebugLoc();<br>
+ auto CL = DL.getLine();<br>
+ auto LineToInstEntry = LineToInst.find(CL);<br>
+ if (LineToInstEntry == LineToInst.end()) {<br>
+ BuildMI(*MBB, *MI, DL, TII->get(AMDGPU::S_NOP))<br>
+ .addImm(0);<br>
+ LineToInst.insert(std::make_pair(CL, MI)); </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ } else {<br>
+ LineToInstEntry->second = MI;<br>
+ }<br>
+ }<br>
+ }<br>
+ for (auto LineToInstEntry = LineToInst.begin();<br>
+ LineToInstEntry != LineToInst.end(); ++LineToInstEntry) {<br></blockquote><div><br>Range for ^<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ auto MBB = LineToInstEntry->second->getParent();<br>
+ auto DL = LineToInstEntry->second->getDebugLoc();<br>
+ MachineBasicBlock::iterator MI = LineToInstEntry->second;<br>
+ ++MI;<br>
+ if (MI != MBB->end()) {<br>
+ BuildMI(*MBB, *MI, DL, TII->get(AMDGPU::S_NOP))<br>
+ .addImm(0);<br>
+ }<br>
+ }<br>
+ MachineBasicBlock &MBB = MF.front();<br>
+ MachineInstr &MI = MBB.front();<br>
+ BuildMI(MBB, MI, DebugLoc(), TII->get(AMDGPU::S_NOP))<br>
+ .addImm(0);<br>
+<br>
+ return true;<br>
+}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>