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

[Sergei Larin]

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&r1=
> 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