[PATCHES] R600/SI: VI fixes
Marek Olšák
maraeo at gmail.com
Thu Jan 15 08:12:03 PST 2015
Hi Tom,
I reworked the patch because it contained a bug and I also needed a
separate helper for converting pseudos to native opcodes that deals
with the various return values nicely.
Changes: I took most of the code from hasVALU32BitEncoding dealing
with pseudos and put it into new AMDGPUInstInfo::pseudoToMCOpcode. I
also moved all dependencies from
AMDGPUMCInstLower to AMDGPUInstInfo. Then I updated both
hasVALU32BitEncoding and AMDGPUMCInstLower to use the new function.
On top of that, it reports an error if a pseudo cannot be converted to
a native opcode during codegen. This can happen due to a bug, and we
definitely don't want to emit pseudos into the binary.
Please review.
Thanks,
Marek
On Wed, Jan 14, 2015 at 12:05 AM, Tom Stellard <tom at stellard.net> wrote:
> On Tue, Jan 13, 2015 at 05:10:32PM +0100, Marek Olšák wrote:
>> Hi Tom and Matt,
>>
>> Attached is a new version of patch "R600/SI: Don't shrink instructions
>> whose e32 encoding doesn't exist".
>>
>> I decided to update the hasVALU32BitEncoding function and use that
>> instead. It also explains the difference between getMCOpcode returning
>> -1 and (uint16_t)-1.
>>
>
> LGTM.
>
>> Marek
>>
>
>> From 9991f2e5aa099aac44a07af75a1760cdbd0b3a42 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Marek=20Ol=C5=A1=C3=A1k?= <marek.olsak at amd.com>
>> Date: Mon, 15 Dec 2014 13:09:36 +0100
>> Subject: [PATCH] R600/SI: Don't shrink instructions whose e32 encoding doesn't
>> exist
>>
>> v2: modify hasVALU32BitEncoding instead
>> ---
>> lib/Target/R600/AMDGPUMCInstLower.cpp | 16 +++++++---------
>> lib/Target/R600/AMDGPUMCInstLower.h | 7 +++----
>> lib/Target/R600/SIInstrInfo.cpp | 14 +++++++++++++-
>> lib/Target/R600/SIShrinkInstructions.cpp | 9 +++++----
>> 4 files changed, 28 insertions(+), 18 deletions(-)
>>
>> diff --git a/lib/Target/R600/AMDGPUMCInstLower.cpp b/lib/Target/R600/AMDGPUMCInstLower.cpp
>> index 84ee784..3d98175 100644
>> --- a/lib/Target/R600/AMDGPUMCInstLower.cpp
>> +++ b/lib/Target/R600/AMDGPUMCInstLower.cpp
>> @@ -40,7 +40,7 @@ AMDGPUMCInstLower::AMDGPUMCInstLower(MCContext &ctx, const AMDGPUSubtarget &st):
>> { }
>>
>> enum AMDGPUMCInstLower::SISubtarget
>> -AMDGPUMCInstLower::AMDGPUSubtargetToSISubtarget(unsigned Gen) const {
>> +AMDGPUMCInstLower::AMDGPUSubtargetToSISubtarget(unsigned Gen) {
>> switch (Gen) {
>> default:
>> return AMDGPUMCInstLower::SI;
>> @@ -49,19 +49,17 @@ AMDGPUMCInstLower::AMDGPUSubtargetToSISubtarget(unsigned Gen) const {
>> }
>> }
>>
>> -unsigned AMDGPUMCInstLower::getMCOpcode(unsigned MIOpcode) const {
>> +int AMDGPUMCInstLower::getMCOpcode(unsigned MIOpcode, unsigned Gen) {
>>
>> - int MCOpcode = AMDGPU::getMCOpcode(MIOpcode,
>> - AMDGPUSubtargetToSISubtarget(ST.getGeneration()));
>> - if (MCOpcode == -1)
>> - MCOpcode = MIOpcode;
>> -
>> - return MCOpcode;
>> + return AMDGPU::getMCOpcode(MIOpcode, AMDGPUSubtargetToSISubtarget(Gen));
>> }
>>
>> void AMDGPUMCInstLower::lower(const MachineInstr *MI, MCInst &OutMI) const {
>>
>> - OutMI.setOpcode(getMCOpcode(MI->getOpcode()));
>> + int MIOpcode = MI->getOpcode();
>> + int MCOpcode = getMCOpcode(MIOpcode, ST.getGeneration());
>> +
>> + OutMI.setOpcode(MCOpcode != -1 ? MCOpcode : MIOpcode);
>>
>> for (const MachineOperand &MO : MI->explicit_operands()) {
>> MCOperand MCOp;
>> diff --git a/lib/Target/R600/AMDGPUMCInstLower.h b/lib/Target/R600/AMDGPUMCInstLower.h
>> index 0ae4d11..56dc3e3 100644
>> --- a/lib/Target/R600/AMDGPUMCInstLower.h
>> +++ b/lib/Target/R600/AMDGPUMCInstLower.h
>> @@ -31,10 +31,7 @@ class AMDGPUMCInstLower {
>>
>> /// Convert a member of the AMDGPUSubtarget::Generation enum to the
>> /// SISubtarget enum.
>> - enum SISubtarget AMDGPUSubtargetToSISubtarget(unsigned Gen) const;
>> -
>> - /// Get the MC opcode for this MachineInstr.
>> - unsigned getMCOpcode(unsigned MIOpcode) const;
>> + static enum SISubtarget AMDGPUSubtargetToSISubtarget(unsigned Gen);
>>
>> public:
>> AMDGPUMCInstLower(MCContext &ctx, const AMDGPUSubtarget &ST);
>> @@ -42,6 +39,8 @@ public:
>> /// \brief Lower a MachineInstr to an MCInst
>> void lower(const MachineInstr *MI, MCInst &OutMI) const;
>>
>> + /// Get the MC opcode for this MachineInstr.
>> + static int getMCOpcode(unsigned MIOpcode, unsigned Gen);
>> };
>>
>> } // End namespace llvm
>> diff --git a/lib/Target/R600/SIInstrInfo.cpp b/lib/Target/R600/SIInstrInfo.cpp
>> index 6454ad5..4105929 100644
>> --- a/lib/Target/R600/SIInstrInfo.cpp
>> +++ b/lib/Target/R600/SIInstrInfo.cpp
>> @@ -1055,7 +1055,19 @@ bool SIInstrInfo::canFoldOffset(unsigned OffsetSize, unsigned AS) const {
>> }
>>
>> bool SIInstrInfo::hasVALU32BitEncoding(unsigned Opcode) const {
>> - return AMDGPU::getVOPe32(Opcode) != -1;
>> + int Op32 = AMDGPU::getVOPe32(Opcode);
>> + if (Op32 == -1)
>> + return false;
>> +
>> + int MCOp = AMDGPU::getMCOpcode(Op32, RI.ST.getGeneration());
>> +
>> + // -1 means that Op32 is already a native instruction.
>> + if (MCOp == -1)
>> + return true;
>> +
>> + // (uint16_t)-1 means that Op32 is a pseudo instruction that has
>> + // no encoding in the given subtarget generation.
>> + return MCOp != (uint16_t)-1;
>> }
>>
>> bool SIInstrInfo::hasModifiers(unsigned Opcode) const {
>> diff --git a/lib/Target/R600/SIShrinkInstructions.cpp b/lib/Target/R600/SIShrinkInstructions.cpp
>> index e97f934..e8cf57b 100644
>> --- a/lib/Target/R600/SIShrinkInstructions.cpp
>> +++ b/lib/Target/R600/SIShrinkInstructions.cpp
>> @@ -10,6 +10,7 @@
>> //
>>
>> #include "AMDGPU.h"
>> +#include "AMDGPUMCInstLower.h"
>> #include "AMDGPUSubtarget.h"
>> #include "SIInstrInfo.h"
>> #include "llvm/ADT/Statistic.h"
>> @@ -213,13 +214,13 @@ bool SIShrinkInstructions::runOnMachineFunction(MachineFunction &MF) {
>> continue;
>> }
>>
>> - int Op32 = AMDGPU::getVOPe32(MI.getOpcode());
>> -
>> - // Op32 could be -1 here if we started with an instruction that had a
>> + // getVOPe32 could be -1 here if we started with an instruction that had
>> // a 32-bit encoding and then commuted it to an instruction that did not.
>> - if (Op32 == -1)
>> + if (!TII->hasVALU32BitEncoding(MI.getOpcode()))
>> continue;
>>
>> + int Op32 = AMDGPU::getVOPe32(MI.getOpcode());
>> +
>> if (TII->isVOPC(Op32)) {
>> unsigned DstReg = MI.getOperand(0).getReg();
>> if (TargetRegisterInfo::isVirtualRegister(DstReg)) {
>> --
>> 2.1.0
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-R600-SI-Don-t-shrink-instructions-whose-e32-encoding.patch
Type: text/x-patch
Size: 8402 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150115/d107b45d/attachment.bin>
More information about the llvm-commits
mailing list