[llvm] r188426 - R600/SI: Choose the correct MOV instruction for copying immediates
Tom Stellard
tom at stellard.net
Wed Aug 21 15:10:18 PDT 2013
On Tue, Aug 20, 2013 at 03:27:14PM -0700, Tom Stellard wrote:
> 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.
As it turns out, adding a test case for this was harder that I thought.
When I first wrote this patch it was to fix a bug that was uncovered
while working on the change that became the very next commit. However, the
final version of that change which I committed actually does not hit
this bug.
I was unable to come up with a test case for this bug, but it may be
possible to write a good test case once we have added more features and
optimizations to the backend, so maybe I'll be able to write one sometime
in the future.
-Tom
>
> -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
> >
> _______________________________________________
> 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