<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 10/07/2014 05:11 PM, Tom Stellard
wrote:<br>
</div>
<blockquote cite="mid:20141008001125.GB12203@freedesktop.org"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-unicode">
<pre wrap="">On Tue, Oct 07, 2014 at 12:07:02AM -0700, Matt Arsenault wrote:
</pre>
<blockquote type="cite" style="color: #000000;">
<pre wrap=""><span class="moz-txt-citetags">> </span>
<span class="moz-txt-citetags">> </span>On Oct 3, 2014, at 8:38 AM, Tom Stellard <a moz-do-not-send="true" class="moz-txt-link-rfc2396E" href="mailto:thomas.stellard@amd.com"><thomas.stellard@amd.com></a> wrote:
<span class="moz-txt-citetags">> </span>
</pre>
<blockquote type="cite" style="color: #000000;">
<pre wrap=""><span class="moz-txt-citetags">> > </span>LLVM assumes INSERT_SUBREG will always have register operands, so
<span class="moz-txt-citetags">> > </span>we need to legalize non-register operands, like FrameIndexes, to
<span class="moz-txt-citetags">> > </span>avoid random assertion failures.
<span class="moz-txt-citetags">> > </span>---
<span class="moz-txt-citetags">> > </span>lib/Target/R600/SIISelLowering.cpp | 27 +++++++++++++++++++++++++++
<span class="moz-txt-citetags">> > </span>test/CodeGen/R600/insert_subreg.ll | 15 +++++++++++++++
<span class="moz-txt-citetags">> > </span>2 files changed, 42 insertions(+)
<span class="moz-txt-citetags">> > </span>create mode 100644 test/CodeGen/R600/insert_subreg.ll
<span class="moz-txt-citetags">> > </span>
<span class="moz-txt-citetags">> > </span>diff --git a/lib/Target/R600/SIISelLowering.cpp b/lib/Target/R600/SIISelLowering.cpp
<span class="moz-txt-citetags">> > </span>index f042aaa..4c0d22d 100644
<span class="moz-txt-citetags">> > </span>--- a/lib/Target/R600/SIISelLowering.cpp
<span class="moz-txt-citetags">> > </span>+++ b/lib/Target/R600/SIISelLowering.cpp
<span class="moz-txt-citetags">> > </span>@@ -1923,6 +1923,30 @@ void SITargetLowering::adjustWritemask(MachineSDNode *&Node,
<span class="moz-txt-citetags">> > </span> }
<span class="moz-txt-citetags">> > </span>}
<span class="moz-txt-citetags">> > </span>
<span class="moz-txt-citetags">> > </span>+/// \brief Legalize INSERT_SUBREG instructions with frame index operands.
<span class="moz-txt-citetags">> > </span>+/// LLVM assumes that all INSERT_SUBREG inputs are registers.
<span class="moz-txt-citetags">> > </span>+static void LegalizeInsertSubreg(MachineSDNode *InsertSubreg,
<span class="moz-txt-citetags">> > </span>+ SelectionDAG &DAG) {
</pre>
</blockquote>
<pre wrap=""><span class="moz-txt-citetags">> </span>
<span class="moz-txt-citetags">> </span>Current convention for new code is function names start with a lowercase letter
<span class="moz-txt-citetags">> </span>
</pre>
<blockquote type="cite" style="color: #000000;">
<pre wrap=""><span class="moz-txt-citetags">> > </span>+
<span class="moz-txt-citetags">> > </span>+ assert(InsertSubreg->getMachineOpcode() == AMDGPU::INSERT_SUBREG);
<span class="moz-txt-citetags">> > </span>+
<span class="moz-txt-citetags">> > </span>+ SmallVector <SDValue, 8> Ops;
</pre>
</blockquote>
<pre wrap=""><span class="moz-txt-citetags">> </span>Extra space between SmallVector and <
<span class="moz-txt-citetags">> </span>
</pre>
<blockquote type="cite" style="color: #000000;">
<pre wrap=""><span class="moz-txt-citetags">> > </span>+ for (unsigned i = 0; i < 2; ++i) {
<span class="moz-txt-citetags">> > </span>+ if (!isa<FrameIndexSDNode>(InsertSubreg->getOperand(i))) {
<span class="moz-txt-citetags">> > </span>+ Ops.push_back(InsertSubreg->getOperand(i));
<span class="moz-txt-citetags">> > </span>+ continue;
<span class="moz-txt-citetags">> > </span>+ }
<span class="moz-txt-citetags">> > </span>+
<span class="moz-txt-citetags">> > </span>+ SDLoc DL(InsertSubreg);
<span class="moz-txt-citetags">> > </span>+ Ops.push_back(SDValue(DAG.getMachineNode(AMDGPU::S_MOV_B32, DL,
<span class="moz-txt-citetags">> > </span>+ InsertSubreg->getOperand(i).getValueType(),
<span class="moz-txt-citetags">> > </span>+ InsertSubreg->getOperand(i)), 0));
<span class="moz-txt-citetags">> > </span>+ }
<span class="moz-txt-citetags">> > </span>+
<span class="moz-txt-citetags">> > </span>+ DAG.UpdateNodeOperands(InsertSubreg, Ops[0], Ops[1],
<span class="moz-txt-citetags">> > </span>+ InsertSubreg->getOperand(2));
<span class="moz-txt-citetags">> > </span>+}
<span class="moz-txt-citetags">> > </span>+
<span class="moz-txt-citetags">> > </span>/// \brief Fold the instructions after selecting them.
<span class="moz-txt-citetags">> > </span>SDNode *SITargetLowering::PostISelFolding(MachineSDNode *Node,
<span class="moz-txt-citetags">> > </span> SelectionDAG &DAG) const {
<span class="moz-txt-citetags">> > </span>@@ -1933,6 +1957,9 @@ SDNode *SITargetLowering::PostISelFolding(MachineSDNode *Node,
<span class="moz-txt-citetags">> > </span> if (TII->isMIMG(Node->getMachineOpcode()))
<span class="moz-txt-citetags">> > </span> adjustWritemask(Node, DAG);
<span class="moz-txt-citetags">> > </span>
<span class="moz-txt-citetags">> > </span>+ if (Node->getMachineOpcode() == AMDGPU::INSERT_SUBREG)
<span class="moz-txt-citetags">> > </span>+ LegalizeInsertSubreg(Node, DAG);
</pre>
</blockquote>
<pre wrap=""><span class="moz-txt-citetags">> </span>
<span class="moz-txt-citetags">> </span>I think you can just return here, since there isn’t really anything legalizeOperands needs to do for insert_subreg
<span class="moz-txt-citetags">> </span>
</pre>
<blockquote type="cite" style="color: #000000;">
<pre wrap=""><span class="moz-txt-citetags">> > </span>+
<span class="moz-txt-citetags">> > </span> return legalizeOperands(Node, DAG);
<span class="moz-txt-citetags">> > </span>}
<span class="moz-txt-citetags">> > </span>
<span class="moz-txt-citetags">> > </span>diff --git a/test/CodeGen/R600/insert_subreg.ll b/test/CodeGen/R600/insert_subreg.ll
<span class="moz-txt-citetags">> > </span>new file mode 100644
<span class="moz-txt-citetags">> > </span>index 0000000..c9b22ec
<span class="moz-txt-citetags">> > </span>--- /dev/null
<span class="moz-txt-citetags">> > </span>+++ b/test/CodeGen/R600/insert_subreg.ll
<span class="moz-txt-citetags">> > </span>@@ -0,0 +1,15 @@
<span class="moz-txt-citetags">> > </span>+; RUN: llc -march=r600 -mcpu=SI -mattr=-promote-alloca -verify-machineinstrs < %s 2>&1 | FileCheck %s
<span class="moz-txt-citetags">> > </span>+
<span class="moz-txt-citetags">> > </span>+; Test that INSERT_SUBREG instructions don't have non-register operands after
<span class="moz-txt-citetags">> > </span>+; instruction selection.
</pre>
</blockquote>
<pre wrap=""><span class="moz-txt-citetags">> </span>
<span class="moz-txt-citetags">> </span>If it’s not testing anything no reason to use FileCheck
<span class="moz-txt-citetags">> </span>
</pre>
<blockquote type="cite" style="color: #000000;">
<pre wrap=""><span class="moz-txt-citetags">> > </span>+
<span class="moz-txt-citetags">> > </span>+; Make sure this doesn't crash
<span class="moz-txt-citetags">> > </span>+; CHECK-LABEL: test:
<span class="moz-txt-citetags">> > </span>+define void @test(i64 addrspace(1)* %out) {
<span class="moz-txt-citetags">> > </span>+entry:
<span class="moz-txt-citetags">> > </span>+ %tmp0 = alloca [16 x i32]
<span class="moz-txt-citetags">> > </span>+ %tmp1 = ptrtoint [16 x i32]* %tmp0 to i32
<span class="moz-txt-citetags">> > </span>+ %tmp2 = sext i32 %tmp1 to i64
<span class="moz-txt-citetags">> > </span>+ store i64 %tmp2, i64 addrspace(1)* %out
<span class="moz-txt-citetags">> > </span>+ ret void
<span class="moz-txt-citetags">> > </span>+}
<span class="moz-txt-citetags">> > </span>--
<span class="moz-txt-citetags">> > </span>1.8.5.5
<span class="moz-txt-citetags">> > </span>
</pre>
</blockquote>
</blockquote>
<pre wrap="">Hi Matt,
Here is an updated patch, plus a similar fix for CopyToReg nodes.
-Tom
</pre>
</div>
</blockquote>
LGTM<br>
</body>
</html>