llvm dev - Oct 2012 - [LLVMdev] [llvm-commits] [llvm] r162770 - in /llvm/trunk: include/llvm/CodeGen/MachineOperand.h lib/CodeGen/MachineInstr.cpp

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

On Oct 29, 2012, at 3:28 PM, "Sergei Larin" <slarin at
codeaurora.org> wrote:
> 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.
I don't really have anything to add to the commit message. There aren't
plans to allow target flags on register operands.

Any particular reason you can't use immediates?

/jakob
>>>> 
>>>> 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.

Jakob,

  I just wanted to know _if_ I can use it for regs for some other custom
markers. Your answer is rather sufficient.
If the real need will arrive, I'll come back to this discussion. 
  Thank you.

Sergei

---
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation

> -----Original Message-----
> From: Jakob Stoklund Olesen [mailto:stoklund at 2pi.dk]
> Sent: Monday, October 29, 2012 5:33 PM
> To: Sergei Larin
> Cc: 'Arnold Schwaighofer'; llvmdev at cs.uiuc.edu
> Subject: Re: [LLVMdev] [llvm-commits] [llvm] r162770 - in /llvm/trunk:
> include/llvm/CodeGen/MachineOperand.h lib/CodeGen/MachineInstr.cpp
> 
> 
> On Oct 29, 2012, at 3:28 PM, "Sergei Larin" <slarin at
codeaurora.org>
> wrote:
> 
> > 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.
> 
> I don't really have anything to add to the commit message. There
aren't
> plans to allow target flags on register operands.
> 
> Any particular reason you can't use immediates?
> 
> /jakob
> 
> >>>>
> >>>> 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.
> 
>