<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">No worries. Thanks for the effort = ).<div><br></div><div>Michael</div><div><br><div><div>On Aug 21, 2013, at 3:10 PM, Tom Stellard <<a href="mailto:tom@stellard.net">tom@stellard.net</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">On Tue, Aug 20, 2013 at 03:27:14PM -0700, Tom Stellard wrote:<br><blockquote type="cite">On Sat, Aug 17, 2013 at 06:57:21PM -0700, Michael Gottesman wrote:<br><blockquote type="cite">From the commit message, it seems that this patch is fixing a bug (I.e. emitting an incorrect mov instruction). If my understanding is correct, is it possible to make a test case for this?<br><br></blockquote><br>Yes, I can add a test case for this.<br></blockquote><br>As it turns out, adding a test case for this was harder that I thought.<br>When I first wrote this patch it was to fix a bug that was uncovered<br>while working on the change that became the very next commit. However, the<br>final version of that change which I committed actually does not hit<br>this bug.<br><br>I was unable to come up with a test case for this bug, but it may be<br>possible to write a good test case once we have added more features and<br>optimizations to the backend, so maybe I'll be able to write one sometime<br>in the future.<br><br>-Tom<br><br><blockquote type="cite"><br>-Tom<br><br><blockquote type="cite">Just trying to be helpful,<br>Michael<br><br>On Aug 16, 2013, at 5:00 PM, Tom Stellard <<a href="mailto:tom@stellard.net">tom@stellard.net</a>> wrote:<br><br><blockquote type="cite">On Thu, Aug 15, 2013 at 11:16:43AM +0400, Alexey Samsonov wrote:<br><blockquote type="cite">Hi Tom!<br><br>This commit breaks our ASan bootstrap bot.<br>The test<br>LLVM :: CodeGen/R600/indirect-addressing-si.ll<br>fails with the following report:<br>=================================================================<br>==5720==ERROR: AddressSanitizer: global-buffer-overflow on address<br>0x000002f35010 at pc 0xf0a316 bp 0x7fff36b19a90 sp 0x7fff36b19a88<br>READ of size 2 at 0x000002f35010 thread T0<br> #0 0xf0a315 in (anonymous<br>namespace)::AMDGPUDAGToDAGISel::getOperandRegClass(llvm::SDNode*, unsigned<br>int) const llvm/lib/Target/R600/AMDGPUISelDAGToDAG.cpp:118<br> #1 0xf04780 in (anonymous<br>namespace)::AMDGPUDAGToDAGISel::CheckNodePredicate(llvm::SDNode*, unsigned<br>int) const llvm_build_asan/lib/Target/R600/AMDGPUGenDAGISel.inc:14035<br> #2 0x191500a in llvm::SelectionDAGISel::SelectCodeCommon(llvm::SDNode*,<br>unsigned char const*, unsigned int)<br>llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:1976<br> #3 0xeff4eb in SelectCode<br>llvm_build_asan/lib/Target/R600/AMDGPUGenDAGISel.inc:13723<br> #4 0xeff4eb in (anonymous<br>namespace)::AMDGPUDAGToDAGISel::Select(llvm::SDNode*)<br>llvm/lib/Target/R600/AMDGPUISelDAGToDAG.cpp:496<br> #5 0x1906f07 in llvm::SelectionDAGISel::DoInstructionSelection()<br>llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:790<br> #6 0x1903823 in llvm::SelectionDAGISel::CodeGenAndEmitDAG()<br>llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:691<br> #7 0x18fefad in<br>llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&)<br>llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:1134<br> #8 0x18f898e in<br>llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&)<br>llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:387<br> #9 0x1e3c1cb in<br>llvm::MachineFunctionPass::runOnFunction(llvm::Function&)<br>llvm/lib/CodeGen/MachineFunctionPass.cpp:33<br> #10 0x279c42c in llvm::FPPassManager::runOnFunction(llvm::Function&)<br>llvm/lib/IR/PassManager.cpp:1530<br> #11 0x279c9f5 in llvm::FPPassManager::runOnModule(llvm::Module&)<br>llvm/lib/IR/PassManager.cpp:1550<br> #12 0x279d21b in llvm::MPPassManager::runOnModule(llvm::Module&)<br>llvm/lib/IR/PassManager.cpp:1608<br> #13 0x279e433 in llvm::PassManagerImpl::run(llvm::Module&)<br>llvm/lib/IR/PassManager.cpp:1703<br> #14 0x53351f in compileModule llvm/tools/llc/llc.cpp:376<br> #15 0x53351f in main llvm/tools/llc/llc.cpp:197<br> #16 0x7fee21eb276c in __libc_start_main<br>/build/buildd/eglibc-2.15/csu/libc-start.c:226<br> #17 0x52effc in _start (llvm_build_asan/bin/llc+0x52effc)<br>0x000002f35010 is located 48 bytes to the left of global variable<br>'llvm::OperandInfo69' from<br>'llvm/lib/Target/R600/MCTargetDesc/AMDGPUMCTargetDesc.cpp' (0x2f35040) of<br>size 48<br>0x000002f35010 is located 0 bytes to the right of global variable<br>'llvm::OperandInfo68' from<br>'llvm/lib/Target/R600/MCTargetDesc/AMDGPUMCTargetDesc.cpp' (0x2f34fe0) of<br>size 48<br>SUMMARY: AddressSanitizer: global-buffer-overflow<br>llvm/lib/Target/R600/AMDGPUISelDAGToDAG.cpp:118 (anonymous<br>namespace)::AMDGPUDAGToDAGISel::getOperandRegClass(llvm::SDNode*, unsigned<br>int) const<br><br><br>I've commited a tentative fix to bring the bot back in r188448. Not sure if<br>the fix is correct - probably you should fix the caller and add an assert<br>to getOperandRegClass function.<br><br></blockquote><br>Your fix actually looks good to me as is. Thanks for catching this.<br><br>-Tom<br><br><blockquote type="cite"><br>On Thu, Aug 15, 2013 at 3:24 AM, Tom Stellard <<a href="mailto:thomas.stellard@amd.com">thomas.stellard@amd.com</a>>wrote:<br><br><blockquote type="cite">Author: tstellar<br>Date: Wed Aug 14 18:24:24 2013<br>New Revision: 188426<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=188426&view=rev">http://llvm.org/viewvc/llvm-project?rev=188426&view=rev</a><br>Log:<br>R600/SI: Choose the correct MOV instruction for copying immediates<br><br>The instruction selector will now try to infer the destination register<br>so it can decided whether to use V_MOV_B32 or S_MOV_B32 when copying<br>immediates.<br><br>Modified:<br> llvm/trunk/lib/Target/R600/AMDGPUISelDAGToDAG.cpp<br> llvm/trunk/lib/Target/R600/SIInstrInfo.td<br> llvm/trunk/lib/Target/R600/SIInstructions.td<br> llvm/trunk/lib/Target/R600/SIRegisterInfo.cpp<br> llvm/trunk/lib/Target/R600/SIRegisterInfo.h<br><br>Modified: llvm/trunk/lib/Target/R600/AMDGPUISelDAGToDAG.cpp<br>URL:<br><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/R600/AMDGPUISelDAGToDAG.cpp?rev=188426&r1=188425&r2=188426&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/R600/AMDGPUISelDAGToDAG.cpp?rev=188426&r1=188425&r2=188426&view=diff</a><br><br>==============================================================================<br>--- llvm/trunk/lib/Target/R600/AMDGPUISelDAGToDAG.cpp (original)<br>+++ llvm/trunk/lib/Target/R600/AMDGPUISelDAGToDAG.cpp Wed Aug 14 18:24:24<br>2013<br>@@ -77,6 +77,7 @@ private:<br> bool isLocalLoad(const LoadSDNode *N) const;<br> bool isRegionLoad(const LoadSDNode *N) const;<br><br>+ const TargetRegisterClass *getOperandRegClass(SDNode *N, unsigned OpNo)<br>const;<br> bool SelectGlobalValueConstantOffset(SDValue Addr, SDValue& IntPtr);<br> bool SelectGlobalValueVariableOffset(SDValue Addr,<br> SDValue &BaseReg, SDValue& Offset);<br>@@ -102,6 +103,34 @@ AMDGPUDAGToDAGISel::AMDGPUDAGToDAGISel(T<br>AMDGPUDAGToDAGISel::~AMDGPUDAGToDAGISel() {<br>}<br><br>+/// \brief Determine the register class for \p OpNo<br>+/// \returns The register class of the virtual register that will be used<br>for<br>+/// the given operand number \OpNo or NULL if the register class cannot be<br>+/// determined.<br>+const TargetRegisterClass *AMDGPUDAGToDAGISel::getOperandRegClass(SDNode<br>*N,<br>+ unsigned OpNo)<br>const {<br>+ if (!N->isMachineOpcode()) {<br>+ return NULL;<br>+ }<br>+ switch (N->getMachineOpcode()) {<br>+ default: {<br>+ const MCInstrDesc &Desc =<br>TM.getInstrInfo()->get(N->getMachineOpcode());<br>+ int RegClass = Desc.OpInfo[Desc.getNumDefs() + OpNo].RegClass;<br>+ if (RegClass == -1) {<br>+ return NULL;<br>+ }<br>+ return TM.getRegisterInfo()->getRegClass(RegClass);<br>+ }<br>+ case AMDGPU::REG_SEQUENCE: {<br>+ const TargetRegisterClass *SuperRC =<br>TM.getRegisterInfo()->getRegClass(<br>+<br>cast<ConstantSDNode>(N->getOperand(0))->getZExtValue());<br>+ unsigned SubRegIdx =<br>+ dyn_cast<ConstantSDNode>(N->getOperand(OpNo +<br>1))->getZExtValue();<br>+ return TM.getRegisterInfo()->getSubClassWithSubReg(SuperRC,<br>SubRegIdx);<br>+ }<br>+ }<br>+}<br>+<br>SDValue AMDGPUDAGToDAGISel::getSmallIPtrImm(unsigned int Imm) {<br> return CurDAG->getTargetConstant(Imm, MVT::i32);<br>}<br><br>Modified: llvm/trunk/lib/Target/R600/SIInstrInfo.td<br>URL:<br>http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/R600/SIInstrInfo.td?rev=188426&r1=188425&r2=188426&view=diff<br><br>==============================================================================<br>--- llvm/trunk/lib/Target/R600/SIInstrInfo.td (original)<br>+++ llvm/trunk/lib/Target/R600/SIInstrInfo.td Wed Aug 14 18:24:24 2013<br>@@ -58,6 +58,22 @@ class InlineImm <ValueType vt> : PatLeaf<br> (*(const SITargetLowering *)getTargetLowering()).analyzeImmediate(N)<br>== 0;<br>}]>;<br><br>+class SGPRImm <dag frag> : PatLeaf<frag, [{<br>+ if (TM.getSubtarget<AMDGPUSubtarget>().getGeneration() <<br>+ AMDGPUSubtarget::SOUTHERN_ISLANDS) {<br>+ return false;<br>+ }<br>+ const SIRegisterInfo *SIRI =<br>+ static_cast<const<br>SIRegisterInfo*>(TM.getRegisterInfo());<br>+ for (SDNode::use_iterator U = N->use_begin(), E = SDNode::use_end();<br>+ U != E; ++U) {<br>+ if (SIRI->isSGPRClass(getOperandRegClass(*U, U.getOperandNo()))) {<br>+ return true;<br>+ }<br>+ }<br>+ return false;<br>+}]>;<br>+<br><br>//===----------------------------------------------------------------------===//<br>// SI assembler operands<br><br>//===----------------------------------------------------------------------===//<br><br>Modified: llvm/trunk/lib/Target/R600/SIInstructions.td<br>URL:<br>http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/R600/SIInstructions.td?rev=188426&r1=188425&r2=188426&view=diff<br><br>==============================================================================<br>--- llvm/trunk/lib/Target/R600/SIInstructions.td (original)<br>+++ llvm/trunk/lib/Target/R600/SIInstructions.td Wed Aug 14 18:24:24 2013<br>@@ -1583,6 +1583,16 @@ def : Pat <<br>/********** ================== **********/<br><br>def : Pat <<br>+ (SGPRImm<(i32 imm)>:$imm),<br>+ (S_MOV_B32 imm:$imm)<br>+>;<br>+<br>+def : Pat <<br>+ (SGPRImm<(f32 fpimm)>:$imm),<br>+ (S_MOV_B32 fpimm:$imm)<br>+>;<br>+<br>+def : Pat <<br> (i32 imm:$imm),<br> (V_MOV_B32_e32 imm:$imm)<br><blockquote type="cite">;<br></blockquote><br>Modified: llvm/trunk/lib/Target/R600/SIRegisterInfo.cpp<br>URL:<br>http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/R600/SIRegisterInfo.cpp?rev=188426&r1=188425&r2=188426&view=diff<br><br>==============================================================================<br>--- llvm/trunk/lib/Target/R600/SIRegisterInfo.cpp (original)<br>+++ llvm/trunk/lib/Target/R600/SIRegisterInfo.cpp Wed Aug 14 18:24:24 2013<br>@@ -70,3 +70,14 @@ const TargetRegisterClass *SIRegisterInf<br> }<br> return NULL;<br>}<br>+<br>+bool SIRegisterInfo::isSGPRClass(const TargetRegisterClass *RC) const {<br>+ if (!RC) {<br>+ return false;<br>+ }<br>+ return RC == &AMDGPU::SReg_32RegClass ||<br>+ RC == &AMDGPU::SReg_64RegClass ||<br>+ RC == &AMDGPU::SReg_128RegClass ||<br>+ RC == &AMDGPU::SReg_256RegClass ||<br>+ RC == &AMDGPU::SReg_512RegClass;<br>+}<br><br>Modified: llvm/trunk/lib/Target/R600/SIRegisterInfo.h<br>URL:<br>http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/R600/SIRegisterInfo.h?rev=188426&r1=188425&r2=188426&view=diff<br><br>==============================================================================<br>--- llvm/trunk/lib/Target/R600/SIRegisterInfo.h (original)<br>+++ llvm/trunk/lib/Target/R600/SIRegisterInfo.h Wed Aug 14 18:24:24 2013<br>@@ -45,6 +45,9 @@ struct SIRegisterInfo : public AMDGPUReg<br> /// \brief Return the 'base' register class for this register.<br> /// e.g. SGPR0 => SReg_32, VGPR => VReg_32 SGPR0_SGPR1 -> SReg_32, etc.<br> const TargetRegisterClass *getPhysRegClass(unsigned Reg) const;<br>+<br>+ /// \returns true if this class contains only SGPR registers<br>+ bool isSGPRClass(const TargetRegisterClass *RC) const;<br>};<br><br>} // End namespace llvm<br><br><br>_______________________________________________<br>llvm-commits mailing list<br>llvm-commits@cs.uiuc.edu<br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br><br></blockquote><br><br><br>--<span class="Apple-converted-space"> </span><br>Alexey Samsonov, MSK<br></blockquote><br><blockquote type="cite">_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br></blockquote><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br></blockquote><br></blockquote>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></blockquote></div></blockquote></div><br></div></body></html>