[llvm] r223268 - [Hexagon] Converting member InstrDesc to static variable.
Rafael Espíndola
rafael.espindola at gmail.com
Mon Dec 8 10:56:23 PST 2014
On 8 December 2014 at 11:20, Colin LeMahieu <colinl at codeaurora.org> wrote:
> That's a good question it's entirely possible this code has never been used in the fashion you've described.
>
> From what I've seen of the usage of this variable it only accesses TSFlags which I assumed would be the same across contexts. This variable was created in lieu of calling createHexagonMCInstrInfo every time we need to access TSFlags at the MC level.
Not sure. What do other backends do?
> Do you have a recommendation as to where this variable should be placed? If we only access TSFlags will it be an issue?
There can still be a race condition in the set, no?
>
> -----Original Message-----
> From: Rafael Espíndola [mailto:rafael.espindola at gmail.com]
> Sent: Sunday, December 07, 2014 10:54 PM
> To: Colin LeMahieu
> Cc: llvm-commits
> Subject: Re: [llvm] r223268 - [Hexagon] Converting member InstrDesc to static variable.
>
> I don't think this is valid. Think of a program using two libraries.
> Both libraries use llvm with different contexts and don't know each other exists.
>
> On 3 December 2014 at 16:40, Colin LeMahieu <colinl at codeaurora.org> wrote:
>> Author: colinl
>> Date: Wed Dec 3 15:40:25 2014
>> New Revision: 223268
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=223268&view=rev
>> Log:
>> [Hexagon] Converting member InstrDesc to static variable.
>>
>> Modified:
>> llvm/trunk/lib/Target/Hexagon/HexagonMCInstLower.cpp
>> llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInst.cpp
>> llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInst.h
>> llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCTargetDesc.cpp
>>
>> Modified: llvm/trunk/lib/Target/Hexagon/HexagonMCInstLower.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/Hexa
>> gonMCInstLower.cpp?rev=223268&r1=223267&r2=223268&view=diff
>> ======================================================================
>> ========
>> --- llvm/trunk/lib/Target/Hexagon/HexagonMCInstLower.cpp (original)
>> +++ llvm/trunk/lib/Target/Hexagon/HexagonMCInstLower.cpp Wed Dec 3
>> +++ 15:40:25 2014
>> @@ -42,7 +42,6 @@ static MCOperand GetSymbolRef(const Mach void
>> llvm::HexagonLowerToMC(const MachineInstr* MI, HexagonMCInst& MCI,
>> HexagonAsmPrinter& AP) {
>> MCI.setOpcode(MI->getOpcode());
>> - MCI.setDesc(MI->getDesc());
>>
>> for (unsigned i = 0, e = MI->getNumOperands(); i < e; i++) {
>> const MachineOperand &MO = MI->getOperand(i);
>>
>> Modified: llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInst.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/MCTa
>> rgetDesc/HexagonMCInst.cpp?rev=223268&r1=223267&r2=223268&view=diff
>> ======================================================================
>> ========
>> --- llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInst.cpp
>> (original)
>> +++ llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInst.cpp Wed
>> +++ Dec 3 15:40:25 2014
>> @@ -18,8 +18,10 @@
>>
>> using namespace llvm;
>>
>> -HexagonMCInst::HexagonMCInst() : MCInst(), MCID(nullptr) {}
>> -HexagonMCInst::HexagonMCInst(MCInstrDesc const &mcid) : MCInst(),
>> MCID(&mcid) {}
>> +std::unique_ptr <MCInstrInfo const> HexagonMCInst::MCII;
>> +
>> +HexagonMCInst::HexagonMCInst() : MCInst() {}
>> +HexagonMCInst::HexagonMCInst(MCInstrDesc const &mcid) : MCInst() {}
>>
>> void HexagonMCInst::AppendImplicitOperands(MCInst &MCI) {
>> MCI.addOperand(MCOperand::CreateImm(0));
>> @@ -75,16 +77,18 @@ unsigned HexagonMCInst::getUnits(const H
>> return (IS->getUnits());
>> }
>>
>> +MCInstrDesc const& HexagonMCInst::getDesc() const { return
>> +(MCII->get(getOpcode())); }
>> +
>> // Return the Hexagon ISA class for the insn.
>> unsigned HexagonMCInst::getType() const {
>> - const uint64_t F = MCID->TSFlags;
>> + const uint64_t F = getDesc().TSFlags;
>>
>> return ((F >> HexagonII::TypePos) & HexagonII::TypeMask); }
>>
>> // Return whether the insn is an actual insn.
>> bool HexagonMCInst::isCanon() const {
>> - return (!MCID->isPseudo() && !isPrefix() &&
>> + return (!getDesc().isPseudo() && !isPrefix() &&
>> getType() != HexagonII::TypeENDLOOP); }
>>
>> @@ -95,25 +99,25 @@ bool HexagonMCInst::isPrefix() const {
>>
>> // Return whether the insn is solo, i.e., cannot be in a packet.
>> bool HexagonMCInst::isSolo() const {
>> - const uint64_t F = MCID->TSFlags;
>> + const uint64_t F = getDesc().TSFlags;
>> return ((F >> HexagonII::SoloPos) & HexagonII::SoloMask); }
>>
>> // Return whether the insn is a new-value consumer.
>> bool HexagonMCInst::isNewValue() const {
>> - const uint64_t F = MCID->TSFlags;
>> + const uint64_t F = getDesc().TSFlags;
>> return ((F >> HexagonII::NewValuePos) & HexagonII::NewValueMask);
>> }
>>
>> // Return whether the instruction is a legal new-value producer.
>> bool HexagonMCInst::hasNewValue() const {
>> - const uint64_t F = MCID->TSFlags;
>> + const uint64_t F = getDesc().TSFlags;
>> return ((F >> HexagonII::hasNewValuePos) &
>> HexagonII::hasNewValueMask); }
>>
>> // Return the operand that consumes or produces a new value.
>> const MCOperand &HexagonMCInst::getNewValue() const {
>> - const uint64_t F = MCID->TSFlags;
>> + const uint64_t F = getDesc().TSFlags;
>> const unsigned O =
>> (F >> HexagonII::NewValueOpPos) & HexagonII::NewValueOpMask;
>> const MCOperand &MCO = getOperand(O); @@ -161,31 +165,31 @@ bool
>> HexagonMCInst::isConstExtended(void
>>
>> // Return whether the instruction must be always extended.
>> bool HexagonMCInst::isExtended(void) const {
>> - const uint64_t F = MCID->TSFlags;
>> + const uint64_t F = getDesc().TSFlags;
>> return (F >> HexagonII::ExtendedPos) & HexagonII::ExtendedMask; }
>>
>> // Return true if the instruction may be extended based on the operand value.
>> bool HexagonMCInst::isExtendable(void) const {
>> - const uint64_t F = MCID->TSFlags;
>> + const uint64_t F = getDesc().TSFlags;
>> return (F >> HexagonII::ExtendablePos) & HexagonII::ExtendableMask;
>> }
>>
>> // Return number of bits in the constant extended operand.
>> unsigned HexagonMCInst::getBitCount(void) const {
>> - const uint64_t F = MCID->TSFlags;
>> + const uint64_t F = getDesc().TSFlags;
>> return ((F >> HexagonII::ExtentBitsPos) &
>> HexagonII::ExtentBitsMask); }
>>
>> // Return constant extended operand number.
>> unsigned short HexagonMCInst::getCExtOpNum(void) const {
>> - const uint64_t F = MCID->TSFlags;
>> + const uint64_t F = getDesc().TSFlags;
>> return ((F >> HexagonII::ExtendableOpPos) &
>> HexagonII::ExtendableOpMask); }
>>
>> // Return whether the operand can be constant extended.
>> bool HexagonMCInst::isOperandExtended(const unsigned short
>> OperandNum) const {
>> - const uint64_t F = MCID->TSFlags;
>> + const uint64_t F = getDesc().TSFlags;
>> return ((F >> HexagonII::ExtendableOpPos) & HexagonII::ExtendableOpMask) ==
>> OperandNum;
>> }
>> @@ -193,7 +197,7 @@ bool HexagonMCInst::isOperandExtended(co
>> // Return the min value that a constant extendable operand can have
>> // without being extended.
>> int HexagonMCInst::getMinValue(void) const {
>> - const uint64_t F = MCID->TSFlags;
>> + const uint64_t F = getDesc().TSFlags;
>> unsigned isSigned =
>> (F >> HexagonII::ExtentSignedPos) & HexagonII::ExtentSignedMask;
>> unsigned bits = (F >> HexagonII::ExtentBitsPos) &
>> HexagonII::ExtentBitsMask; @@ -207,7 +211,7 @@ int
>> HexagonMCInst::getMinValue(void) con // Return the max value that a
>> constant extendable operand can have // without being extended.
>> int HexagonMCInst::getMaxValue(void) const {
>> - const uint64_t F = MCID->TSFlags;
>> + const uint64_t F = getDesc().TSFlags;
>> unsigned isSigned =
>> (F >> HexagonII::ExtentSignedPos) & HexagonII::ExtentSignedMask;
>> unsigned bits = (F >> HexagonII::ExtentBitsPos) &
>> HexagonII::ExtentBitsMask;
>>
>> Modified: llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInst.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/MCTa
>> rgetDesc/HexagonMCInst.h?rev=223268&r1=223267&r2=223268&view=diff
>> ======================================================================
>> ========
>> --- llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInst.h
>> (original)
>> +++ llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInst.h Wed Dec
>> +++ 3 15:40:25 2014
>> @@ -17,14 +17,16 @@
>> #include "HexagonTargetMachine.h"
>> #include "llvm/MC/MCInst.h"
>>
>> +#include <memory>
>> +
>> +extern "C" void LLVMInitializeHexagonTargetMC();
>> namespace llvm {
>> class MCOperand;
>>
>> class HexagonMCInst : public MCInst {
>> - // MCID is set during instruction lowering.
>> - // It is needed in order to access TSFlags for
>> - // use in checking MC instruction properties.
>> - const MCInstrDesc *MCID;
>> + friend void ::LLVMInitializeHexagonTargetMC();
>> + // Used to access TSFlags
>> + static std::unique_ptr <MCInstrInfo const> MCII;
>>
>> public:
>> explicit HexagonMCInst();
>> @@ -55,8 +57,7 @@ public:
>> // Return the Hexagon ISA class for the insn.
>> unsigned getType() const;
>>
>> - void setDesc(const MCInstrDesc &mcid) { MCID = &mcid; };
>> - const MCInstrDesc &getDesc(void) const { return *MCID; };
>> + MCInstrDesc const &getDesc() const;
>>
>> // Return whether the insn is an actual insn.
>> bool isCanon() const;
>>
>> Modified:
>> llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCTargetDesc.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/MCTa
>> rgetDesc/HexagonMCTargetDesc.cpp?rev=223268&r1=223267&r2=223268&view=d
>> iff
>> ======================================================================
>> ========
>> --- llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCTargetDesc.cpp
>> (original)
>> +++ llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCTargetDesc.cpp
>> +++ Wed Dec 3 15:40:25 2014
>> @@ -14,6 +14,7 @@
>> #include "HexagonMCTargetDesc.h"
>> #include "HexagonMCAsmInfo.h"
>> #include "MCTargetDesc/HexagonInstPrinter.h"
>> +#include "MCTargetDesc/HexagonMCInst.h"
>> #include "llvm/MC/MCCodeGenInfo.h"
>> #include "llvm/MC/MCELFStreamer.h"
>> #include "llvm/MC/MCInstrInfo.h"
>> @@ -115,6 +116,7 @@ extern "C" void LLVMInitializeHexagonTar
>> // Register the MC instruction info.
>> TargetRegistry::RegisterMCInstrInfo(TheHexagonTarget,
>> createHexagonMCInstrInfo);
>> + HexagonMCInst::MCII.reset (createHexagonMCInstrInfo());
>>
>> // Register the MC register info.
>> TargetRegistry::RegisterMCRegInfo(TheHexagonTarget,
>>
>>
>> _______________________________________________
>> 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