[llvm] r223268 - [Hexagon] Converting member InstrDesc to static variable.
Colin LeMahieu
colinl at codeaurora.org
Mon Dec 8 08:20:15 PST 2014
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.
Do you have a recommendation as to where this variable should be placed? If we only access TSFlags will it be an issue?
-----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