[llvm] r188426 - R600/SI: Choose the correct MOV instruction for copying immediates
Michael Gottesman
mgottesman at apple.com
Wed Aug 21 16:25:41 PDT 2013
No worries. Thanks for the effort = ).
Michael
On Aug 21, 2013, at 3:10 PM, Tom Stellard <tom at stellard.net> wrote:
> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130821/9399d364/attachment.html>
More information about the llvm-commits
mailing list