[llvm] r188426 - R600/SI: Choose the correct MOV instruction for copying immediates
Michael Gottesman
mgottesman at apple.com
Sat Aug 17 18:57:21 PDT 2013
>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?
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