[llvm] r188426 - R600/SI: Choose the correct MOV instruction for copying immediates
Tom Stellard
tom at stellard.net
Tue Aug 20 15:27:14 PDT 2013
On Sat, Aug 17, 2013 at 06:57:21PM -0700, Michael Gottesman wrote:
> 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?
>
Yes, I can add a test case for this.
-Tom
> Just trying to be helpful,
> Michael
>
> On Aug 16, 2013, at 5:00 PM, Tom Stellard <tom at stellard.net> wrote:
>
> > On Thu, Aug 15, 2013 at 11:16:43AM +0400, Alexey Samsonov wrote:
> >> Hi Tom!
> >>
> >> This commit breaks our ASan bootstrap bot.
> >> The test
> >> LLVM :: CodeGen/R600/indirect-addressing-si.ll
> >> fails with the following report:
> >> =================================================================
> >> ==5720==ERROR: AddressSanitizer: global-buffer-overflow on address
> >> 0x000002f35010 at pc 0xf0a316 bp 0x7fff36b19a90 sp 0x7fff36b19a88
> >> READ of size 2 at 0x000002f35010 thread T0
> >> #0 0xf0a315 in (anonymous
> >> namespace)::AMDGPUDAGToDAGISel::getOperandRegClass(llvm::SDNode*, unsigned
> >> int) const llvm/lib/Target/R600/AMDGPUISelDAGToDAG.cpp:118
> >> #1 0xf04780 in (anonymous
> >> namespace)::AMDGPUDAGToDAGISel::CheckNodePredicate(llvm::SDNode*, unsigned
> >> int) const llvm_build_asan/lib/Target/R600/AMDGPUGenDAGISel.inc:14035
> >> #2 0x191500a in llvm::SelectionDAGISel::SelectCodeCommon(llvm::SDNode*,
> >> unsigned char const*, unsigned int)
> >> llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:1976
> >> #3 0xeff4eb in SelectCode
> >> llvm_build_asan/lib/Target/R600/AMDGPUGenDAGISel.inc:13723
> >> #4 0xeff4eb in (anonymous
> >> namespace)::AMDGPUDAGToDAGISel::Select(llvm::SDNode*)
> >> llvm/lib/Target/R600/AMDGPUISelDAGToDAG.cpp:496
> >> #5 0x1906f07 in llvm::SelectionDAGISel::DoInstructionSelection()
> >> llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:790
> >> #6 0x1903823 in llvm::SelectionDAGISel::CodeGenAndEmitDAG()
> >> llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:691
> >> #7 0x18fefad in
> >> llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&)
> >> llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:1134
> >> #8 0x18f898e in
> >> llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&)
> >> llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:387
> >> #9 0x1e3c1cb in
> >> llvm::MachineFunctionPass::runOnFunction(llvm::Function&)
> >> llvm/lib/CodeGen/MachineFunctionPass.cpp:33
> >> #10 0x279c42c in llvm::FPPassManager::runOnFunction(llvm::Function&)
> >> llvm/lib/IR/PassManager.cpp:1530
> >> #11 0x279c9f5 in llvm::FPPassManager::runOnModule(llvm::Module&)
> >> llvm/lib/IR/PassManager.cpp:1550
> >> #12 0x279d21b in llvm::MPPassManager::runOnModule(llvm::Module&)
> >> llvm/lib/IR/PassManager.cpp:1608
> >> #13 0x279e433 in llvm::PassManagerImpl::run(llvm::Module&)
> >> llvm/lib/IR/PassManager.cpp:1703
> >> #14 0x53351f in compileModule llvm/tools/llc/llc.cpp:376
> >> #15 0x53351f in main llvm/tools/llc/llc.cpp:197
> >> #16 0x7fee21eb276c in __libc_start_main
> >> /build/buildd/eglibc-2.15/csu/libc-start.c:226
> >> #17 0x52effc in _start (llvm_build_asan/bin/llc+0x52effc)
> >> 0x000002f35010 is located 48 bytes to the left of global variable
> >> 'llvm::OperandInfo69' from
> >> 'llvm/lib/Target/R600/MCTargetDesc/AMDGPUMCTargetDesc.cpp' (0x2f35040) of
> >> size 48
> >> 0x000002f35010 is located 0 bytes to the right of global variable
> >> 'llvm::OperandInfo68' from
> >> 'llvm/lib/Target/R600/MCTargetDesc/AMDGPUMCTargetDesc.cpp' (0x2f34fe0) of
> >> size 48
> >> SUMMARY: AddressSanitizer: global-buffer-overflow
> >> llvm/lib/Target/R600/AMDGPUISelDAGToDAG.cpp:118 (anonymous
> >> namespace)::AMDGPUDAGToDAGISel::getOperandRegClass(llvm::SDNode*, unsigned
> >> int) const
> >>
> >>
> >> I've commited a tentative fix to bring the bot back in r188448. Not sure if
> >> the fix is correct - probably you should fix the caller and add an assert
> >> to getOperandRegClass function.
> >>
> >
> > Your fix actually looks good to me as is. Thanks for catching this.
> >
> > -Tom
> >
> >>
> >> On Thu, Aug 15, 2013 at 3:24 AM, Tom Stellard <thomas.stellard at amd.com>wrote:
> >>
> >>> Author: tstellar
> >>> Date: Wed Aug 14 18:24:24 2013
> >>> New Revision: 188426
> >>>
> >>> URL: http://llvm.org/viewvc/llvm-project?rev=188426&view=rev
> >>> Log:
> >>> R600/SI: Choose the correct MOV instruction for copying immediates
> >>>
> >>> The instruction selector will now try to infer the destination register
> >>> so it can decided whether to use V_MOV_B32 or S_MOV_B32 when copying
> >>> immediates.
> >>>
> >>> Modified:
> >>> llvm/trunk/lib/Target/R600/AMDGPUISelDAGToDAG.cpp
> >>> llvm/trunk/lib/Target/R600/SIInstrInfo.td
> >>> llvm/trunk/lib/Target/R600/SIInstructions.td
> >>> llvm/trunk/lib/Target/R600/SIRegisterInfo.cpp
> >>> llvm/trunk/lib/Target/R600/SIRegisterInfo.h
> >>>
> >>> Modified: llvm/trunk/lib/Target/R600/AMDGPUISelDAGToDAG.cpp
> >>> URL:
> >>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/R600/AMDGPUISelDAGToDAG.cpp?rev=188426&r1=188425&r2=188426&view=diff
> >>>
> >>> ==============================================================================
> >>> --- llvm/trunk/lib/Target/R600/AMDGPUISelDAGToDAG.cpp (original)
> >>> +++ llvm/trunk/lib/Target/R600/AMDGPUISelDAGToDAG.cpp Wed Aug 14 18:24:24
> >>> 2013
> >>> @@ -77,6 +77,7 @@ private:
> >>> bool isLocalLoad(const LoadSDNode *N) const;
> >>> bool isRegionLoad(const LoadSDNode *N) const;
> >>>
> >>> + const TargetRegisterClass *getOperandRegClass(SDNode *N, unsigned OpNo)
> >>> const;
> >>> bool SelectGlobalValueConstantOffset(SDValue Addr, SDValue& IntPtr);
> >>> bool SelectGlobalValueVariableOffset(SDValue Addr,
> >>> SDValue &BaseReg, SDValue& Offset);
> >>> @@ -102,6 +103,34 @@ AMDGPUDAGToDAGISel::AMDGPUDAGToDAGISel(T
> >>> AMDGPUDAGToDAGISel::~AMDGPUDAGToDAGISel() {
> >>> }
> >>>
> >>> +/// \brief Determine the register class for \p OpNo
> >>> +/// \returns The register class of the virtual register that will be used
> >>> for
> >>> +/// the given operand number \OpNo or NULL if the register class cannot be
> >>> +/// determined.
> >>> +const TargetRegisterClass *AMDGPUDAGToDAGISel::getOperandRegClass(SDNode
> >>> *N,
> >>> + unsigned OpNo)
> >>> const {
> >>> + if (!N->isMachineOpcode()) {
> >>> + return NULL;
> >>> + }
> >>> + switch (N->getMachineOpcode()) {
> >>> + default: {
> >>> + const MCInstrDesc &Desc =
> >>> TM.getInstrInfo()->get(N->getMachineOpcode());
> >>> + int RegClass = Desc.OpInfo[Desc.getNumDefs() + OpNo].RegClass;
> >>> + if (RegClass == -1) {
> >>> + return NULL;
> >>> + }
> >>> + return TM.getRegisterInfo()->getRegClass(RegClass);
> >>> + }
> >>> + case AMDGPU::REG_SEQUENCE: {
> >>> + const TargetRegisterClass *SuperRC =
> >>> TM.getRegisterInfo()->getRegClass(
> >>> +
> >>> cast<ConstantSDNode>(N->getOperand(0))->getZExtValue());
> >>> + unsigned SubRegIdx =
> >>> + dyn_cast<ConstantSDNode>(N->getOperand(OpNo +
> >>> 1))->getZExtValue();
> >>> + return TM.getRegisterInfo()->getSubClassWithSubReg(SuperRC,
> >>> SubRegIdx);
> >>> + }
> >>> + }
> >>> +}
> >>> +
> >>> SDValue AMDGPUDAGToDAGISel::getSmallIPtrImm(unsigned int Imm) {
> >>> return CurDAG->getTargetConstant(Imm, MVT::i32);
> >>> }
> >>>
> >>> Modified: llvm/trunk/lib/Target/R600/SIInstrInfo.td
> >>> URL:
> >>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/R600/SIInstrInfo.td?rev=188426&r1=188425&r2=188426&view=diff
> >>>
> >>> ==============================================================================
> >>> --- llvm/trunk/lib/Target/R600/SIInstrInfo.td (original)
> >>> +++ llvm/trunk/lib/Target/R600/SIInstrInfo.td Wed Aug 14 18:24:24 2013
> >>> @@ -58,6 +58,22 @@ class InlineImm <ValueType vt> : PatLeaf
> >>> (*(const SITargetLowering *)getTargetLowering()).analyzeImmediate(N)
> >>> == 0;
> >>> }]>;
> >>>
> >>> +class SGPRImm <dag frag> : PatLeaf<frag, [{
> >>> + if (TM.getSubtarget<AMDGPUSubtarget>().getGeneration() <
> >>> + AMDGPUSubtarget::SOUTHERN_ISLANDS) {
> >>> + return false;
> >>> + }
> >>> + const SIRegisterInfo *SIRI =
> >>> + static_cast<const
> >>> SIRegisterInfo*>(TM.getRegisterInfo());
> >>> + for (SDNode::use_iterator U = N->use_begin(), E = SDNode::use_end();
> >>> + U != E; ++U) {
> >>> + if (SIRI->isSGPRClass(getOperandRegClass(*U, U.getOperandNo()))) {
> >>> + return true;
> >>> + }
> >>> + }
> >>> + return false;
> >>> +}]>;
> >>> +
> >>>
> >>> //===----------------------------------------------------------------------===//
> >>> // SI assembler operands
> >>>
> >>> //===----------------------------------------------------------------------===//
> >>>
> >>> Modified: llvm/trunk/lib/Target/R600/SIInstructions.td
> >>> URL:
> >>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/R600/SIInstructions.td?rev=188426&r1=188425&r2=188426&view=diff
> >>>
> >>> ==============================================================================
> >>> --- llvm/trunk/lib/Target/R600/SIInstructions.td (original)
> >>> +++ llvm/trunk/lib/Target/R600/SIInstructions.td Wed Aug 14 18:24:24 2013
> >>> @@ -1583,6 +1583,16 @@ def : Pat <
> >>> /********** ================== **********/
> >>>
> >>> def : Pat <
> >>> + (SGPRImm<(i32 imm)>:$imm),
> >>> + (S_MOV_B32 imm:$imm)
> >>> +>;
> >>> +
> >>> +def : Pat <
> >>> + (SGPRImm<(f32 fpimm)>:$imm),
> >>> + (S_MOV_B32 fpimm:$imm)
> >>> +>;
> >>> +
> >>> +def : Pat <
> >>> (i32 imm:$imm),
> >>> (V_MOV_B32_e32 imm:$imm)
> >>>> ;
> >>>
> >>> Modified: llvm/trunk/lib/Target/R600/SIRegisterInfo.cpp
> >>> URL:
> >>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/R600/SIRegisterInfo.cpp?rev=188426&r1=188425&r2=188426&view=diff
> >>>
> >>> ==============================================================================
> >>> --- llvm/trunk/lib/Target/R600/SIRegisterInfo.cpp (original)
> >>> +++ llvm/trunk/lib/Target/R600/SIRegisterInfo.cpp Wed Aug 14 18:24:24 2013
> >>> @@ -70,3 +70,14 @@ const TargetRegisterClass *SIRegisterInf
> >>> }
> >>> return NULL;
> >>> }
> >>> +
> >>> +bool SIRegisterInfo::isSGPRClass(const TargetRegisterClass *RC) const {
> >>> + if (!RC) {
> >>> + return false;
> >>> + }
> >>> + return RC == &AMDGPU::SReg_32RegClass ||
> >>> + RC == &AMDGPU::SReg_64RegClass ||
> >>> + RC == &AMDGPU::SReg_128RegClass ||
> >>> + RC == &AMDGPU::SReg_256RegClass ||
> >>> + RC == &AMDGPU::SReg_512RegClass;
> >>> +}
> >>>
> >>> Modified: llvm/trunk/lib/Target/R600/SIRegisterInfo.h
> >>> URL:
> >>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/R600/SIRegisterInfo.h?rev=188426&r1=188425&r2=188426&view=diff
> >>>
> >>> ==============================================================================
> >>> --- llvm/trunk/lib/Target/R600/SIRegisterInfo.h (original)
> >>> +++ llvm/trunk/lib/Target/R600/SIRegisterInfo.h Wed Aug 14 18:24:24 2013
> >>> @@ -45,6 +45,9 @@ struct SIRegisterInfo : public AMDGPUReg
> >>> /// \brief Return the 'base' register class for this register.
> >>> /// e.g. SGPR0 => SReg_32, VGPR => VReg_32 SGPR0_SGPR1 -> SReg_32, etc.
> >>> const TargetRegisterClass *getPhysRegClass(unsigned Reg) const;
> >>> +
> >>> + /// \returns true if this class contains only SGPR registers
> >>> + bool isSGPRClass(const TargetRegisterClass *RC) const;
> >>> };
> >>>
> >>> } // End namespace llvm
> >>>
> >>>
> >>> _______________________________________________
> >>> llvm-commits mailing list
> >>> llvm-commits at cs.uiuc.edu
> >>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >>>
> >>
> >>
> >>
> >> --
> >> Alexey Samsonov, MSK
> >
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list