[LLVMdev] [llvm-commits] [llvm] r162770 - in /llvm/trunk: include/llvm/CodeGen/MachineOperand.h lib/CodeGen/MachineInstr.cpp
Sergei Larin
slarin at codeaurora.org
Mon Oct 29 15:28:58 PDT 2012
Arnold,
I wanted to hear from Jacob is the original patch in question still needed,
since our use of this field could surpass const extenders and could
potentially include MO_Register.
Jacob,
Can you please comment? Thanks.
Sergei
---
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation
> -----Original Message-----
> From: Arnold Schwaighofer [mailto:arnolds at codeaurora.org]
> Sent: Monday, October 29, 2012 1:02 PM
> To: Sergei Larin
> Cc: Jakob Stoklund Olesen; llvmdev at cs.uiuc.edu
> Subject: Re: [LLVMdev] [llvm-commits] [llvm] r162770 - in /llvm/trunk:
> include/llvm/CodeGen/MachineOperand.h lib/CodeGen/MachineInstr.cpp
>
> Hi Sergei,
>
> our use of target flags will be on immediate register operands if I am
> not mistaken (and if not we can always encode it as such)?
>
> I guess you are refering to the hexagon backend needing to distinguish
> between instances of an instruction that uses a constant value that can
> fit into the 4 byte of the instruction and one that encodes the
> immediate in an extra instruction slot (what we call a constant
> extended instruction).
> We can encode that with a target flag on the immediate machine operand.
> So the patch you quoted is not a problem.
>
> The quoted patch only prohibits the use of flags on register operands
> (that is MO.isReg()==true) because some of the code in CodeGen that
> deals with register operands does not appropriately handle target flags
> on the register operand (it might not transfer the information from an
> "old" operand to an "updated" operand).
>
> I guess I am asking you to clarify what you mean by heavy use of
> TargetFlags on MachineOperands. What specifically do you plan to do?
>
> Jakob,
> TargetFlags on immediate machine operands introduced as early as ISel
> Lowering should not be a problem, right?
>
>
> Thanks,
> Arnold
>
> On Mon, Oct 29, 2012 at 12:17 PM, Sergei Larin <slarin at codeaurora.org>
> wrote:
> >
> > Jakob and anyone else who might be interested...
> >
> > Base on this patch back in August, I sense some need to double
> check
> > with you whether it is OK to start making a heavy use of
> > MachineOperand TargetFlags?
> > We do seem to have a compelling reason for it in Hexagon, and I
> wanted
> > to make sure that it is OK with everyone. I plan to use it for
> > attributing target specific info to MOs and in more general case to
> > MIs that those MOs belongs to.
> > Part of my question would be - is it still unsafe to use it for
> > MO_Register and if so, then why?
> >
> > Thanks a lot.
> >
> > Sergei
> >
> > ---
> > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> > hosted by The Linux Foundation
> >
> >
> >> -----Original Message-----
> >> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> >> bounces at cs.uiuc.edu] On Behalf Of Jakob Stoklund Olesen
> >> Sent: Tuesday, August 28, 2012 1:06 PM
> >> To: llvm-commits at cs.uiuc.edu
> >> Subject: [llvm-commits] [llvm] r162770 - in /llvm/trunk:
> >> include/llvm/CodeGen/MachineOperand.h lib/CodeGen/MachineInstr.cpp
> >>
> >> Author: stoklund
> >> Date: Tue Aug 28 13:05:48 2012
> >> New Revision: 162770
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=162770&view=rev
> >> Log:
> >> Don't allow TargetFlags on MO_Register MachineOperands.
> >>
> >> Register operands are manipulated by a lot of target-independent
> >> code, and it is not always possible to preserve target flags. That
> >> means it is not safe to use target flags on register operands.
> >>
> >> None of the targets in the tree are using register operand target
> >> flags.
> >> External targets should be using immediate operands to annotate
> >> instructions with operand modifiers.
> >>
> >> Modified:
> >> llvm/trunk/include/llvm/CodeGen/MachineOperand.h
> >> llvm/trunk/lib/CodeGen/MachineInstr.cpp
> >>
> >> Modified: llvm/trunk/include/llvm/CodeGen/MachineOperand.h
> >> URL: http://llvm.org/viewvc/llvm-
> >>
> project/llvm/trunk/include/llvm/CodeGen/MachineOperand.h?rev=162770&r
> >> 1=
> >> 162769&r2=162770&view=diff
> >>
> =====================================================================
> >> ==
> >> =======
> >> --- llvm/trunk/include/llvm/CodeGen/MachineOperand.h (original)
> >> +++ llvm/trunk/include/llvm/CodeGen/MachineOperand.h Tue Aug 28
> >> 13:05:48
> >> +++ 2012
> >> @@ -60,12 +60,15 @@
> >> /// union.
> >> unsigned char OpKind; // MachineOperandType
> >>
> >> - /// SubReg - Subregister number, only valid for MO_Register. A
> >> value of 0
> >> - /// indicates the MO_Register has no subReg.
> >> - unsigned char SubReg;
> >> -
> >> - /// TargetFlags - This is a set of target-specific operand flags.
> >> - unsigned char TargetFlags;
> >> + // This union is discriminated by OpKind.
> >> + union {
> >> + /// SubReg - Subregister number, only valid for MO_Register. A
> >> value of 0
> >> + /// indicates the MO_Register has no subReg.
> >> + unsigned char SubReg;
> >> +
> >> + /// TargetFlags - This is a set of target-specific operand
> flags.
> >> + unsigned char TargetFlags;
> >> + };
> >>
> >> /// IsDef/IsImp/IsKill/IsDead flags - These are only valid for
> >> MO_Register
> >> /// operands.
> >> @@ -176,9 +179,17 @@
> >> ///
> >> MachineOperandType getType() const { return
> >> (MachineOperandType)OpKind; }
> >>
> >> - unsigned char getTargetFlags() const { return TargetFlags; }
> >> - void setTargetFlags(unsigned char F) { TargetFlags = F; }
> >> - void addTargetFlag(unsigned char F) { TargetFlags |= F; }
> >> + unsigned char getTargetFlags() const {
> >> + return isReg() ? 0 : TargetFlags; } void
> >> + setTargetFlags(unsigned char F) {
> >> + assert(!isReg() && "Register operands can't have target
> flags");
> >> + TargetFlags = F;
> >> + }
> >> + void addTargetFlag(unsigned char F) {
> >> + assert(!isReg() && "Register operands can't have target
> flags");
> >> + TargetFlags |= F;
> >> + }
> >>
> >>
> >> /// getParent - Return the instruction that this operand belongs
> to.
> >>
> >> Modified: llvm/trunk/lib/CodeGen/MachineInstr.cpp
> >> URL: http://llvm.org/viewvc/llvm-
> >>
> project/llvm/trunk/lib/CodeGen/MachineInstr.cpp?rev=162770&r1=162769&
> >> r2
> >> =162770&view=diff
> >>
> =====================================================================
> >> ==
> >> =======
> >> --- llvm/trunk/lib/CodeGen/MachineInstr.cpp (original)
> >> +++ llvm/trunk/lib/CodeGen/MachineInstr.cpp Tue Aug 28 13:05:48 2012
> >> @@ -208,8 +208,8 @@
> >> hash_code llvm::hash_value(const MachineOperand &MO) {
> >> switch (MO.getType()) {
> >> case MachineOperand::MO_Register:
> >> - return hash_combine(MO.getType(), MO.getTargetFlags(),
> >> MO.getReg(),
> >> - MO.getSubReg(), MO.isDef());
> >> + // Register operands don't have target flags.
> >> + return hash_combine(MO.getType(), MO.getReg(), MO.getSubReg(),
> >> + MO.isDef());
> >> case MachineOperand::MO_Immediate:
> >> return hash_combine(MO.getType(), MO.getTargetFlags(),
> >> MO.getImm());
> >> case MachineOperand::MO_CImmediate:
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> > _______________________________________________
> > LLVM Developers mailing list
> > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
More information about the llvm-dev
mailing list