[AMDGPU] AsmParser: Support for sext() modifier in SDWA. Some code cleaning in AMDGPUOperand.

[SamWot]

Summary

sext() modifier is supported in SDWA instructions only for integer operands. Spec is unclear should integer operands support abs and neg modifiers with sext - for now they are not supported.
Renamed InputModsWithNoDefault to FloatInputMods. Added SextInputMods for operands that support sext() modifier.
Added AMDGPUOperand::Modifier struct to handle register and immediate modifiers.
Code cleaning in AMDGPUOperand class: organize method in groups (render-, predicate-methods...).

[artem.tamazov]

The change introduces some new error messages. It would be good to create tests for those. You may discover (and then fix) something unexpected.

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

73 ↗

(On Diff #59566)

After 1st glance, I would replace has/getSext... by has/getInteger...
After second, recommend replacing "Float" by "Floating" or "Fp". "Float" may be misunderstood as 32-bit FP type only.

90–95 ↗

(On Diff #59566)

Please comment out why floating modifiers take precedence over integer ones.

382 ↗

(On Diff #59566)

This construct asserts cleaner. +Typo in the last word.

assert(... &&
  "float (abs, neg) and sext modifiers should not be used simultaneously");

2382 ↗

(On Diff #59566)

floating-point

lib/Target/AMDGPU/SIDefines.h

86 ↗

(On Diff #59566)

Suspicious. If this is correct, explain why.

[arsenm]

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

91–93 ↗

(On Diff #59566)

no return after else

169–170 ↗

(On Diff #59566)

C++ style comments

[SamWot]

Fixess for review. Renamed FloatInputMods to FPInputMods and SextInputMods to IntInputMods.

[artem.tamazov]

In D20968#450828, @SamWot wrote:

Fixess...

Fixess is a nice-looking girl which likes correcting problems in the code, right? ))

[artem.tamazov]

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

73 ↗

(On Diff #59862)

I would better use "Integer", not "Int".

83 ↗

(On Diff #59862)

Still getSext..., not getInteger... Perhaps Sext is Ok but explain why, please.

90–95 ↗

(On Diff #59862)

Not done, as far as I see.

lib/Target/AMDGPU/SIDefines.h

86 ↗

(On Diff #59862)

Not done.

[SamWot]

I forgot to submit my previous comments.

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

73 ↗

(On Diff #59862)

I prefer Int because it is not that long as Integer. Otherwise method names would be much longer. Also if we use FP for floating point then usage of Int as acronim for Integer looks better.

90–95 ↗

(On Diff #59566)

Floating modifiers have precedence because they are used more often. FP and sext modifiers can't be set immediately so this doesn't affect correctness.

91–93 ↗

(On Diff #59566)

I'm not sure that I understood your comment correctly. There is no need for return after this if-else construct because it covers all possible situations. If you are speaking about coding standatd (http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return) then I don't see what is better way to write this code fragment.

lib/Target/AMDGPU/SIDefines.h

86 ↗

(On Diff #59566)

This is correct.
This just specifies that sext() modifier is defined by first bit in immedieate operand. Sext and abs/neg can't be set simultaneously so no problem arises.

[SamWot]

Some fixess.

[artem.tamazov]

Looks good enough, but I recommend addressing my comments prior committing this.

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

73 ↗

(On Diff #59872)

My point was that "int" might be misunderstood as 32-bit integer type (just like "float" above).
Making "code looks better" at the cost of semantical clarity may camouflage design flaws.
Finally, decide yourself.

90–95 ↗

(On Diff #59872)

Did you mean "FP and sext modifiers can't be set simultaneously..."? If yes, then Ok.
However, add comment to the code. Or, assert if both modifiers set (better).

lib/Target/AMDGPU/SIDefines.h

86 ↗

(On Diff #59872)

Please comment that in the code.

[SamWot]

Ping

llvm/trunk/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

 
 public:
   AMDGPUOperand(enum KindTy K) : MCParsedAsmOperand(), Kind(K) {}
 
   MCContext *Ctx;
 
   typedef std::unique_ptr<AMDGPUOperand> Ptr;
 
+  struct Modifiers {
+    bool Abs;
+    bool Neg;
+    bool Sext;
+
+    bool hasFPModifiers() const { return Abs || Neg; }
+    bool hasIntModifiers() const { return Sext; }
+    bool hasModifiers() const { return hasFPModifiers() || hasIntModifiers(); }
+
+    int64_t getFPModifiersOperand() const {
+      int64_t Operand = 0;
+      Operand |= Abs ? SISrcMods::ABS : 0;
+      Operand |= Neg ? SISrcMods::NEG : 0;
+      return Operand;
+    }
+
+    int64_t getIntModifiersOperand() const {
+      int64_t Operand = 0;
+      Operand |= Sext ? SISrcMods::SEXT : 0;
+      return Operand;
+    }
+
+    int64_t getModifiersOperand() const {
+      assert(!(hasFPModifiers() && hasIntModifiers())
+           && "fp and int modifiers should not be used simultaneously");
+      if (hasFPModifiers()) {
+        return getFPModifiersOperand();
+      } else if (hasIntModifiers()) {
+        return getIntModifiersOperand();
+      } else {
+        return 0;
+      }
+    }
+
+    friend raw_ostream &operator <<(raw_ostream &OS, AMDGPUOperand::Modifiers Mods);
+  };
+
   enum ImmTy {
     ImmTyNone,
     ImmTyGDS,
     ImmTyOffen,
     ImmTyIdxen,
     ImmTyAddr64,
     ImmTyOffset,
     ImmTyOffset0,
struct TokOp {
     const char *Data;
     unsigned Length;
   };
 
   struct ImmOp {
     bool IsFPImm;
     ImmTy Type;
     int64_t Val;
-    int Modifiers;
+    Modifiers Mods;
   };
 
   struct RegOp {
     unsigned RegNo;
-    int Modifiers;
+    Modifiers Mods;
     const MCRegisterInfo *TRI;
     const MCSubtargetInfo *STI;
     bool IsForcedVOP3;
   };
 
   union {
     TokOp Tok;
     ImmOp Imm;
     RegOp Reg;
     const MCExpr *Expr;
   };
 
-  void addImmOperands(MCInst &Inst, unsigned N, bool ApplyModifiers = true) const {
-    if (Imm.Type == ImmTyNone && ApplyModifiers && Imm.Modifiers != 0) {
-      // Apply modifiers to immediate value
-      int64_t Val = Imm.Val;
-      bool Negate = Imm.Modifiers & 0x1;
-      bool Abs = Imm.Modifiers & 0x2;
-      if (Imm.IsFPImm) {
-        APFloat F(BitsToFloat(Val));
-        if (Abs) {
-          F.clearSign();
-        }
-        if (Negate) {
-          F.changeSign();
-        }
-        Val = F.bitcastToAPInt().getZExtValue();
-      } else {
-        Val = Abs ? std::abs(Val) : Val;
-        Val = Negate ? -Val : Val;
-      }
-      Inst.addOperand(MCOperand::createImm(Val));
-    } else {
-      Inst.addOperand(MCOperand::createImm(getImm()));
-    }
-  }
-
-  StringRef getToken() const {
-    return StringRef(Tok.Data, Tok.Length);
-  }
-
-  void addRegOperands(MCInst &Inst, unsigned N) const {
-    Inst.addOperand(MCOperand::createReg(AMDGPU::getMCReg(getReg(), *Reg.STI)));
-  }
-
-  void addRegOrImmOperands(MCInst &Inst, unsigned N) const {
-    if (isRegKind())
-      addRegOperands(Inst, N);
-    else
-      addImmOperands(Inst, N);
-  }
-
-  void addRegOrImmWithInputModsOperands(MCInst &Inst, unsigned N) const {
-    if (isRegKind()) {
-      Inst.addOperand(MCOperand::createImm(Reg.Modifiers));
-      addRegOperands(Inst, N);
-    } else {
-      Inst.addOperand(MCOperand::createImm(Imm.Modifiers));
-      addImmOperands(Inst, N, false);
-    }
-  }
-
-  void addSoppBrTargetOperands(MCInst &Inst, unsigned N) const {
-    if (isImm())
-      addImmOperands(Inst, N);
-    else {
-      assert(isExpr());
-      Inst.addOperand(MCOperand::createExpr(Expr));
-    }
-  }
-
   bool isToken() const override {
     return Kind == Token;
   }
 
   bool isImm() const override {
     return Kind == Immediate;
   }
 
   bool isInlinableImm() const {
-    if (!isImm() || Imm.Type != AMDGPUOperand::ImmTyNone /* Only plain
-      immediates are inlinable (e.g. "clamp" attribute is not) */ )
+    if (!isImmTy(ImmTyNone)) {
+      // Only plain immediates are inlinable (e.g. "clamp" attribute is not)
       return false;
+    }
     // TODO: We should avoid using host float here. It would be better to
     // check the float bit values which is what a few other places do.
     // We've had bot failures before due to weird NaN support on mips hosts.
     const float F = BitsToFloat(Imm.Val);
     // TODO: Add 1/(2*pi) for VI
     return (Imm.Val <= 64 && Imm.Val >= -16) ||
            (F == 0.0 || F == 0.5 || F == -0.5 || F == 1.0 || F == -1.0 ||
            F == 2.0 || F == -2.0 || F == 4.0 || F == -4.0);
   }
 
-  int64_t getImm() const {
-    return Imm.Val;
-  }
-
-  enum ImmTy getImmTy() const {
-    assert(isImm());
-    return Imm.Type;
-  }
-
   bool isRegKind() const {
     return Kind == Register;
   }
 
   bool isReg() const override {
-    return Kind == Register && Reg.Modifiers == 0;
+    return isRegKind() && !Reg.Mods.hasModifiers();
   }
 
   bool isRegOrImmWithInputMods() const {
-    return Kind == Register || isInlinableImm();
+    return isRegKind() || isInlinableImm();
   }
 
   bool isImmTy(ImmTy ImmT) const {
     return isImm() && Imm.Type == ImmT;
   }
   
-  bool isClampSI() const {
-    return isImmTy(ImmTyClampSI);
-  }
-
-  bool isOModSI() const {
-    return isImmTy(ImmTyOModSI);
-  }
-
   bool isImmModifier() const {
-    return Kind == Immediate && Imm.Type != ImmTyNone;
-  }
-
-  bool isDMask() const {
-    return isImmTy(ImmTyDMask);
+    return isImm() && Imm.Type != ImmTyNone;
   }
   
+  bool isClampSI() const { return isImmTy(ImmTyClampSI); }
+  bool isOModSI() const { return isImmTy(ImmTyOModSI); }
+  bool isDMask() const { return isImmTy(ImmTyDMask); }
   bool isUNorm() const { return isImmTy(ImmTyUNorm); }
   bool isDA() const { return isImmTy(ImmTyDA); }
   bool isR128() const { return isImmTy(ImmTyUNorm); }
   bool isLWE() const { return isImmTy(ImmTyLWE); }
-
-  bool isMod() const {
-    return isClampSI() || isOModSI();
-  }
-
   bool isOffen() const { return isImmTy(ImmTyOffen); }
   bool isIdxen() const { return isImmTy(ImmTyIdxen); }
   bool isAddr64() const { return isImmTy(ImmTyAddr64); }
   bool isOffset() const { return isImmTy(ImmTyOffset) && isUInt<16>(getImm()); }
   bool isOffset0() const { return isImmTy(ImmTyOffset0) && isUInt<16>(getImm()); }
   bool isOffset1() const { return isImmTy(ImmTyOffset1) && isUInt<8>(getImm()); }
   bool isGDS() const { return isImmTy(ImmTyGDS); }
   bool isGLC() const { return isImmTy(ImmTyGLC); }
   bool isSLC() const { return isImmTy(ImmTySLC); }
   bool isTFE() const { return isImmTy(ImmTyTFE); }
+  bool isBankMask() const { return isImmTy(ImmTyDppBankMask); }
+  bool isRowMask() const { return isImmTy(ImmTyDppRowMask); }
+  bool isBoundCtrl() const { return isImmTy(ImmTyDppBoundCtrl); }
+  bool isSDWADstSel() const { return isImmTy(ImmTySdwaDstSel); }
+  bool isSDWASrc0Sel() const { return isImmTy(ImmTySdwaSrc0Sel); }
+  bool isSDWASrc1Sel() const { return isImmTy(ImmTySdwaSrc1Sel); }
+  bool isSDWADstUnused() const { return isImmTy(ImmTySdwaDstUnused); }
   
-  bool isBankMask() const {
-    return isImmTy(ImmTyDppBankMask);
-  }
-
-  bool isRowMask() const {
-    return isImmTy(ImmTyDppRowMask);
-  }
-
-  bool isBoundCtrl() const {
-    return isImmTy(ImmTyDppBoundCtrl);
-  }
-
-  bool isSDWADstSel() const {
-    return isImmTy(ImmTySdwaDstSel);
-  }
-
-  bool isSDWASrc0Sel() const {
-    return isImmTy(ImmTySdwaSrc0Sel);
-  }
-
-  bool isSDWASrc1Sel() const {
-    return isImmTy(ImmTySdwaSrc1Sel);
-  }
-
-  bool isSDWADstUnused() const {
-    return isImmTy(ImmTySdwaDstUnused);
-  }
-
-  void setModifiers(unsigned Mods) {
-    assert(isReg() || (isImm() && Imm.Modifiers == 0));
-    if (isReg())
-      Reg.Modifiers = Mods;
-    else
-      Imm.Modifiers = Mods;
-  }
-
-  bool hasModifiers() const {
-    assert(isRegKind() || isImm());
-    return isRegKind() ? Reg.Modifiers != 0 : Imm.Modifiers != 0;
-  }
-
-  unsigned getReg() const override {
-    return Reg.RegNo;
+  bool isMod() const {
+    return isClampSI() || isOModSI();
   }
 
   bool isRegOrImm() const {
     return isReg() || isImm();
   }
 
   bool isRegClass(unsigned RCID) const {
     return isReg() && Reg.TRI->getRegClass(RCID).contains(getReg());
public:
   bool isExpr() const {
     return Kind == Expression;
   }
 
   bool isSoppBrTarget() const {
     return isExpr() || isImm();
   }
 
+  bool isSWaitCnt() const;
+  bool isHwreg() const;
+  bool isSendMsg() const;
+  bool isMubufOffset() const;
+  bool isSMRDOffset() const;
+  bool isSMRDLiteralOffset() const;
+  bool isDPPCtrl() const;
+
+  StringRef getToken() const {
+    return StringRef(Tok.Data, Tok.Length);
+  }
+
+  int64_t getImm() const {
+    assert(isImm());
+    return Imm.Val;
+  }
+
+  enum ImmTy getImmTy() const {
+    assert(isImm());
+    return Imm.Type;
+  }
+
+  unsigned getReg() const override {
+    return Reg.RegNo;
+  }
+
   SMLoc getStartLoc() const override {
     return StartLoc;
   }
 
   SMLoc getEndLoc() const override {
     return EndLoc;
   }
 
+  Modifiers getModifiers() const {
+    assert(isRegKind() || isImmTy(ImmTyNone));
+    return isRegKind() ? Reg.Mods : Imm.Mods;
+  }
+
+  void setModifiers(Modifiers Mods) {
+    assert(isRegKind() || isImmTy(ImmTyNone));
+    if (isRegKind())
+      Reg.Mods = Mods;
+    else
+      Imm.Mods = Mods;
+  }
+
+  bool hasModifiers() const {
+    return getModifiers().hasModifiers();
+  }
+  
+  bool hasFPModifiers() const {
+    return getModifiers().hasFPModifiers();
+  }
+
+  bool hasIntModifiers() const {
+    return getModifiers().hasIntModifiers();
+  }
+
+  void addImmOperands(MCInst &Inst, unsigned N, bool ApplyModifiers = true) const {
+    if (isImmTy(ImmTyNone) && ApplyModifiers && Imm.Mods.hasFPModifiers()) {
+      // Apply modifiers to immediate value 
+      int64_t Val = Imm.Val;
+      bool Negate = Imm.Mods.Neg; // Only negate can get here
+      if (Imm.IsFPImm) {
+        APFloat F(BitsToFloat(Val));
+        if (Negate) {
+          F.changeSign();
+        }
+        Val = F.bitcastToAPInt().getZExtValue();
+      } else {
+        Val = Negate ? -Val : Val;
+      }
+      Inst.addOperand(MCOperand::createImm(Val));
+    } else {
+      Inst.addOperand(MCOperand::createImm(getImm()));
+    }
+  }
+
+  void addRegOperands(MCInst &Inst, unsigned N) const {
+    Inst.addOperand(MCOperand::createReg(AMDGPU::getMCReg(getReg(), *Reg.STI)));
+  }
+
+  void addRegOrImmOperands(MCInst &Inst, unsigned N) const {
+    if (isRegKind())
+      addRegOperands(Inst, N);
+    else
+      addImmOperands(Inst, N);
+  }
+
+  void addRegOrImmWithInputModsOperands(MCInst &Inst, unsigned N) const {
+    Modifiers Mods = getModifiers();
+    Inst.addOperand(MCOperand::createImm(Mods.getModifiersOperand()));
+    if (isRegKind()) {
+      addRegOperands(Inst, N);
+    } else {
+      addImmOperands(Inst, N, false);
+    }
+  }
+
+  void addRegOrImmWithFPInputModsOperands(MCInst &Inst, unsigned N) const {
+    assert(!hasIntModifiers());
+    addRegOrImmWithInputModsOperands(Inst, N);
+  }
+
+  void addRegOrImmWithIntInputModsOperands(MCInst &Inst, unsigned N) const {
+    assert(!hasFPModifiers());
+    addRegOrImmWithInputModsOperands(Inst, N);
+  }
+
+  void addSoppBrTargetOperands(MCInst &Inst, unsigned N) const {
+    if (isImm())
+      addImmOperands(Inst, N);
+    else {
+      assert(isExpr());
+      Inst.addOperand(MCOperand::createExpr(Expr));
+    }
+  }
+
   void printImmTy(raw_ostream& OS, ImmTy Type) const {
     switch (Type) {
     case ImmTyNone: OS << "None"; break;
     case ImmTyGDS: OS << "GDS"; break;
     case ImmTyOffen: OS << "Offen"; break;
     case ImmTyIdxen: OS << "Idxen"; break;
     case ImmTyAddr64: OS << "Addr64"; break;
     case ImmTyOffset: OS << "Offset"; break;
void printImmTy(raw_ostream& OS, ImmTy Type) const {
     case ImmTyHwreg: OS << "Hwreg"; break;
     case ImmTySendMsg: OS << "SendMsg"; break;
     }
   }
 
   void print(raw_ostream &OS) const override {
     switch (Kind) {
     case Register:
-      OS << "<register " << getReg() << " mods: " << Reg.Modifiers << '>';
+      OS << "<register " << getReg() << " mods: " << Reg.Mods << '>';
       break;
     case Immediate:
       OS << '<' << getImm();
       if (getImmTy() != ImmTyNone) {
         OS << " type: "; printImmTy(OS, getImmTy());
       }
-      OS << " mods: " << Imm.Modifiers << '>';
+      OS << " mods: " << Imm.Mods << '>';
       break;
     case Token:
       OS << '\'' << getToken() << '\'';
       break;
     case Expression:
       OS << "<expr " << *Expr << '>';
       break;
     }
   }
 
   static AMDGPUOperand::Ptr CreateImm(int64_t Val, SMLoc Loc,
                                       enum ImmTy Type = ImmTyNone,
                                       bool IsFPImm = false) {
     auto Op = llvm::make_unique<AMDGPUOperand>(Immediate);
     Op->Imm.Val = Val;
     Op->Imm.IsFPImm = IsFPImm;
     Op->Imm.Type = Type;
-    Op->Imm.Modifiers = 0;
+    Op->Imm.Mods = {false, false, false};
     Op->StartLoc = Loc;
     Op->EndLoc = Loc;
     return Op;
   }
 
   static AMDGPUOperand::Ptr CreateToken(StringRef Str, SMLoc Loc,
                                         bool HasExplicitEncodingSize = true) {
     auto Res = llvm::make_unique<AMDGPUOperand>(Token);
     Res->Tok.Data = Str.data();
     Res->Tok.Length = Str.size();
     Res->StartLoc = Loc;
     Res->EndLoc = Loc;
     return Res;
   }
 
   static AMDGPUOperand::Ptr CreateReg(unsigned RegNo, SMLoc S,
                                       SMLoc E,
                                       const MCRegisterInfo *TRI,
                                       const MCSubtargetInfo *STI,
                                       bool ForceVOP3) {
     auto Op = llvm::make_unique<AMDGPUOperand>(Register);
     Op->Reg.RegNo = RegNo;
     Op->Reg.TRI = TRI;
     Op->Reg.STI = STI;
-    Op->Reg.Modifiers = 0;
+    Op->Reg.Mods = {false, false, false};
     Op->Reg.IsForcedVOP3 = ForceVOP3;
     Op->StartLoc = S;
     Op->EndLoc = E;
     return Op;
   }
 
   static AMDGPUOperand::Ptr CreateExpr(const class MCExpr *Expr, SMLoc S) {
     auto Op = llvm::make_unique<AMDGPUOperand>(Expression);
     Op->Expr = Expr;
     Op->StartLoc = S;
     Op->EndLoc = S;
     return Op;
   }
-
-  bool isSWaitCnt() const;
-  bool isHwreg() const;
-  bool isSendMsg() const;
-  bool isMubufOffset() const;
-  bool isSMRDOffset() const;
-  bool isSMRDLiteralOffset() const;
-  bool isDPPCtrl() const;
 };
 
+raw_ostream &operator <<(raw_ostream &OS, AMDGPUOperand::Modifiers Mods) {
+  OS << "abs:" << Mods.Abs << " neg: " << Mods.Neg << " sext:" << Mods.Sext;
+  return OS;
+}
+
 class AMDGPUAsmParser : public MCTargetAsmParser {
   const MCInstrInfo &MII;
   MCAsmParser &Parser;
 
   unsigned ForcedEncodingSize;
   bool ForcedDPP;
   bool ForcedSDWA;
 
OperandMatchResultTy parseIntWithPrefix(const char *Prefix,
                                           enum AMDGPUOperand::ImmTy ImmTy = AMDGPUOperand::ImmTyNone,
                                           bool (*ConvertResult)(int64_t&) = 0);
   OperandMatchResultTy parseNamedBit(const char *Name, OperandVector &Operands,
                                      enum AMDGPUOperand::ImmTy ImmTy = AMDGPUOperand::ImmTyNone);
   OperandMatchResultTy parseStringWithPrefix(StringRef Prefix, StringRef &Value);
 
   OperandMatchResultTy parseImm(OperandVector &Operands);
   OperandMatchResultTy parseRegOrImm(OperandVector &Operands);
-  OperandMatchResultTy parseRegOrImmWithInputMods(OperandVector &Operands);
+  OperandMatchResultTy parseRegOrImmWithFPInputMods(OperandVector &Operands);
+  OperandMatchResultTy parseRegOrImmWithIntInputMods(OperandVector &Operands);
 
   void cvtDSOffset01(MCInst &Inst, const OperandVector &Operands);
   void cvtDS(MCInst &Inst, const OperandVector &Operands);
 
   bool parseCnt(int64_t &IntVal);
   OperandMatchResultTy parseSWaitCntOps(OperandVector &Operands);
   OperandMatchResultTy parseHwreg(OperandVector &Operands);
 
public:
   OperandMatchResultTy parseSDWASel(OperandVector &Operands, StringRef Prefix,
                                     AMDGPUOperand::ImmTy Type);
   OperandMatchResultTy parseSDWADstUnused(OperandVector &Operands);
   AMDGPUOperand::Ptr defaultSDWASel(AMDGPUOperand::ImmTy Type) const;
   AMDGPUOperand::Ptr defaultSDWADstSel() const;
   AMDGPUOperand::Ptr defaultSDWASrc0Sel() const;
   AMDGPUOperand::Ptr defaultSDWASrc1Sel() const;
   AMDGPUOperand::Ptr defaultSDWADstUnused() const;
-  void cvtSdwaVop1_mod(MCInst &Inst, const OperandVector &Operands);
-  void cvtSdwaVop1_nomod(MCInst &Inst, const OperandVector &Operands);
-  void cvtSdwaVop2_mod(MCInst &Inst, const OperandVector &Operands);
-  void cvtSdwaVop2_nomod(MCInst &Inst, const OperandVector &Operands);
-  void cvtSDWA(MCInst &Inst, const OperandVector &Operands, bool HasMods,
-               bool IsVOP1);
+  void cvtSdwaVOP1(MCInst &Inst, const OperandVector &Operands);
+  void cvtSdwaVOP2(MCInst &Inst, const OperandVector &Operands);
+  void cvtSDWA(MCInst &Inst, const OperandVector &Operands, bool IsVOP1);
 };
 
 struct OptionalOperand {
   const char *Name;
   AMDGPUOperand::ImmTy Type;
   bool IsBit;
   bool (*ConvertResult)(int64_t&);
 };
if (auto R = parseRegister()) {
     R->Reg.IsForcedVOP3 = isForcedVOP3();
     Operands.push_back(std::move(R));
     return MatchOperand_Success;
   }
   return MatchOperand_ParseFail;
 }
 
 AMDGPUAsmParser::OperandMatchResultTy
-AMDGPUAsmParser::parseRegOrImmWithInputMods(OperandVector &Operands) {
+AMDGPUAsmParser::parseRegOrImmWithFPInputMods(OperandVector &Operands) {
   // XXX: During parsing we can't determine if minus sign means
   // negate-modifier or negative immediate value.
   // By default we suppose it is modifier.
   bool Negate = false, Abs = false, Abs2 = false;
 
   if (getLexer().getKind()== AsmToken::Minus) {
     Parser.Lex();
     Negate = true;
if (getLexer().getKind() == AsmToken::Pipe) {
     Abs = true;
   }
 
   auto Res = parseRegOrImm(Operands);
   if (Res != MatchOperand_Success) {
     return Res;
   }
 
-  unsigned Modifiers = 0;
+  AMDGPUOperand::Modifiers Mods = {false, false, false};
   if (Negate) {
-    Modifiers |= 0x1;
+    Mods.Neg = true;
   }
   if (Abs) {
     if (getLexer().getKind() != AsmToken::Pipe) {
       Error(Parser.getTok().getLoc(), "expected vertical bar");
       return MatchOperand_ParseFail;
     }
     Parser.Lex();
-    Modifiers |= 0x2;
+    Mods.Abs = true;
   }
   if (Abs2) {
     if (getLexer().isNot(AsmToken::RParen)) {
       Error(Parser.getTok().getLoc(), "expected closing parentheses");
       return MatchOperand_ParseFail;
     }
     Parser.Lex();
-    Modifiers |= 0x2;
+    Mods.Abs = true;
   }
 
-  if (Modifiers) {
+  if (Mods.hasFPModifiers()) {
     AMDGPUOperand &Op = static_cast<AMDGPUOperand &>(*Operands.back());
-    Op.setModifiers(Modifiers);
+    Op.setModifiers(Mods);
   }
   return MatchOperand_Success;
 }
 
+AMDGPUAsmParser::OperandMatchResultTy
+AMDGPUAsmParser::parseRegOrImmWithIntInputMods(OperandVector &Operands) {
+  bool Sext = false;
+
+  if (getLexer().getKind() == AsmToken::Identifier && Parser.getTok().getString() == "sext") {
+    Parser.Lex();
+    Sext = true;
+    if (getLexer().isNot(AsmToken::LParen)) {
+      Error(Parser.getTok().getLoc(), "expected left paren after sext");
+      return MatchOperand_ParseFail;
+    }
+    Parser.Lex();
+  }
+
+  auto Res = parseRegOrImm(Operands);
+  if (Res != MatchOperand_Success) {
+    return Res;
+  }
+
+  AMDGPUOperand &Op = static_cast<AMDGPUOperand &>(*Operands.back());
+  if (Op.isImm() && Op.Imm.IsFPImm) {
+    Error(Parser.getTok().getLoc(), "floating point operands not allowed with sext() modifier");
+    return MatchOperand_ParseFail;
+  }
+
+  AMDGPUOperand::Modifiers Mods = {false, false, false};
+  if (Sext) {
+    if (getLexer().isNot(AsmToken::RParen)) {
+      Error(Parser.getTok().getLoc(), "expected closing parentheses");
+      return MatchOperand_ParseFail;
+    }
+    Parser.Lex();
+    Mods.Sext = true;
+  }
+  
+  if (Mods.hasIntModifiers()) {
+    Op.setModifiers(Mods);
+  }
+  return MatchOperand_Success;
+}
 
 unsigned AMDGPUAsmParser::checkTargetMatchPredicate(MCInst &Inst) {
 
   uint64_t TSFlags = MII.get(Inst.getOpcode()).TSFlags;
 
   if ((getForcedEncodingSize() == 32 && (TSFlags & SIInstrFlags::VOP3)) ||
       (getForcedEncodingSize() == 64 && !(TSFlags & SIInstrFlags::VOP3)) ||
       (isForcedDPP() && !(TSFlags & SIInstrFlags::DPP)) ||
void AMDGPUAsmParser::cvtVOP3(MCInst &Inst, const OperandVector &Operands) {
   const MCInstrDesc &Desc = MII.get(Inst.getOpcode());
   for (unsigned J = 0; J < Desc.getNumDefs(); ++J) {
     ((AMDGPUOperand &)*Operands[I++]).addRegOperands(Inst, 1);
   }
 
   for (unsigned E = Operands.size(); I != E; ++I) {
     AMDGPUOperand &Op = ((AMDGPUOperand &)*Operands[I]);
     if (Op.isRegOrImmWithInputMods()) {
-      Op.addRegOrImmWithInputModsOperands(Inst, 2);
+      // only fp modifiers allowed in VOP3
+      Op.addRegOrImmWithFPInputModsOperands(Inst, 2);
     } else if (Op.isImm()) {
       OptionalIdx[Op.getImmTy()] = I;
     } else {
       assert(false);
     }
   }
 
   addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyClampSI);
void AMDGPUAsmParser::cvtDPP(MCInst &Inst, const OperandVector &Operands) {
   for (unsigned J = 0; J < Desc.getNumDefs(); ++J) {
     ((AMDGPUOperand &)*Operands[I++]).addRegOperands(Inst, 1);
   }
 
   for (unsigned E = Operands.size(); I != E; ++I) {
     AMDGPUOperand &Op = ((AMDGPUOperand &)*Operands[I]);
     // Add the register arguments
     if (Op.isRegOrImmWithInputMods()) {
-      // We convert only instructions with modifiers
-      Op.addRegOrImmWithInputModsOperands(Inst, 2);
+      // Only float modifiers supported in DPP
+      Op.addRegOrImmWithFPInputModsOperands(Inst, 2);
     } else if (Op.isDPPCtrl()) {
       Op.addImmOperands(Inst, 1);
     } else if (Op.isImm()) {
       // Handle optional arguments
       OptionalIdx[Op.getImmTy()] = I;
     } else {
       llvm_unreachable("Invalid operand type");
     }

 AMDGPUOperand::Ptr AMDGPUAsmParser::defaultSDWASrc1Sel() const {
   return defaultSDWASel(AMDGPUOperand::ImmTySdwaSrc1Sel);
 }
 
 AMDGPUOperand::Ptr AMDGPUAsmParser::defaultSDWADstUnused() const {
   return AMDGPUOperand::CreateImm(0, SMLoc(), AMDGPUOperand::ImmTySdwaDstUnused);
 }
 
-void AMDGPUAsmParser::cvtSdwaVop1_mod(MCInst &Inst, const OperandVector &Operands) {
-  cvtSDWA(Inst, Operands, true, true);
+void AMDGPUAsmParser::cvtSdwaVOP1(MCInst &Inst, const OperandVector &Operands) {
+  cvtSDWA(Inst, Operands, true);
 }
 
-void AMDGPUAsmParser::cvtSdwaVop1_nomod(MCInst &Inst, const OperandVector &Operands) {
-  cvtSDWA(Inst, Operands, false, true);
-}
-
-void AMDGPUAsmParser::cvtSdwaVop2_mod(MCInst &Inst, const OperandVector &Operands) {
-  cvtSDWA(Inst, Operands, true, false);
-}
-
-void AMDGPUAsmParser::cvtSdwaVop2_nomod(MCInst &Inst, const OperandVector &Operands) {
-  cvtSDWA(Inst, Operands, false, false);
+void AMDGPUAsmParser::cvtSdwaVOP2(MCInst &Inst, const OperandVector &Operands) {
+  cvtSDWA(Inst, Operands, false);
 }
 
 void AMDGPUAsmParser::cvtSDWA(MCInst &Inst, const OperandVector &Operands,
-                              bool HasMods, bool IsVOP1) {
+                              bool IsVOP1) {
   OptionalImmIndexMap OptionalIdx;
 
   unsigned I = 1;
   const MCInstrDesc &Desc = MII.get(Inst.getOpcode());
   for (unsigned J = 0; J < Desc.getNumDefs(); ++J) {
     ((AMDGPUOperand &)*Operands[I++]).addRegOperands(Inst, 1);
   }
 
   for (unsigned E = Operands.size(); I != E; ++I) {
     AMDGPUOperand &Op = ((AMDGPUOperand &)*Operands[I]);
     // Add the register arguments
-    if (!HasMods && Op.isReg()) {
-      Op.addRegOperands(Inst, 1);
-    } else if (HasMods && Op.isRegOrImmWithInputMods()) {
+    if (Op.isRegOrImmWithInputMods()) {
        Op.addRegOrImmWithInputModsOperands(Inst, 2);
     } else if (Op.isImm()) {
       // Handle optional arguments
       OptionalIdx[Op.getImmTy()] = I;
     } else {
       llvm_unreachable("Invalid operand type");
     }
   }
 
-  if (HasMods) {
-    addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyClampSI, 0);
-  }
+  
   if (Inst.getOpcode() == AMDGPU::V_NOP_sdwa) {
     // V_NOP_sdwa has no optional sdwa arguments
     return;
   }
   if (IsVOP1) {
     addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTySdwaDstSel, 6);
     addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTySdwaDstUnused, 2);
     addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTySdwaSrc0Sel, 6);

llvm/trunk/lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.h

private:
   void printDA(const MCInst *MI, unsigned OpNo, raw_ostream &O);
   void printR128(const MCInst *MI, unsigned OpNo, raw_ostream &O);
   void printLWE(const MCInst *MI, unsigned OpNo, raw_ostream &O);
   void printRegOperand(unsigned RegNo, raw_ostream &O);
   void printVOPDst(const MCInst *MI, unsigned OpNo, raw_ostream &O);
   void printImmediate32(uint32_t I, raw_ostream &O);
   void printImmediate64(uint64_t I, raw_ostream &O);
   void printOperand(const MCInst *MI, unsigned OpNo, raw_ostream &O);
-  void printOperandAndMods(const MCInst *MI, unsigned OpNo, raw_ostream &O);
+  void printOperandAndFPInputMods(const MCInst *MI, unsigned OpNo, raw_ostream &O);
+  void printOperandAndIntInputMods(const MCInst *MI, unsigned OpNo, raw_ostream &O);
   void printDPPCtrl(const MCInst *MI, unsigned OpNo, raw_ostream &O);
   void printRowMask(const MCInst *MI, unsigned OpNo, raw_ostream &O);
   void printBankMask(const MCInst *MI, unsigned OpNo, raw_ostream &O);
   void printBoundCtrl(const MCInst *MI, unsigned OpNo, raw_ostream &O);
   void printSDWASel(const MCInst *MI, unsigned OpNo, raw_ostream &O);
   void printSDWADstSel(const MCInst *MI, unsigned OpNo, raw_ostream &O);
   void printSDWASrc0Sel(const MCInst *MI, unsigned OpNo, raw_ostream &O);
   void printSDWASrc1Sel(const MCInst *MI, unsigned OpNo, raw_ostream &O);

llvm/trunk/lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp

void AMDGPUInstPrinter::printOperand(const MCInst *MI, unsigned OpNo,
   } else if (Op.isExpr()) {
     const MCExpr *Exp = Op.getExpr();
     Exp->print(O, &MAI);
   } else {
     O << "/*INV_OP*/";
   }
 }
 
-void AMDGPUInstPrinter::printOperandAndMods(const MCInst *MI, unsigned OpNo,
+void AMDGPUInstPrinter::printOperandAndFPInputMods(const MCInst *MI,
+                                                      unsigned OpNo,
                                                       raw_ostream &O) {
   unsigned InputModifiers = MI->getOperand(OpNo).getImm();
   if (InputModifiers & SISrcMods::NEG)
     O << '-';
   if (InputModifiers & SISrcMods::ABS)
     O << '|';
   printOperand(MI, OpNo + 1, O);
   if (InputModifiers & SISrcMods::ABS)
     O << '|';
 }
 
+void AMDGPUInstPrinter::printOperandAndIntInputMods(const MCInst *MI,
+                                                     unsigned OpNo,
+                                                     raw_ostream &O) {
+  unsigned InputModifiers = MI->getOperand(OpNo).getImm();
+  if (InputModifiers & SISrcMods::SEXT)
+    O << "sext(";
+  printOperand(MI, OpNo + 1, O);
+  if (InputModifiers & SISrcMods::SEXT)
+    O << ')';
+}
+
 
 void AMDGPUInstPrinter::printDPPCtrl(const MCInst *MI, unsigned OpNo,
                                              raw_ostream &O) {
   unsigned Imm = MI->getOperand(OpNo).getImm();
   if (Imm <= 0x0ff) {
     O << " quad_perm:[";
     O << formatDec(Imm & 0x3)         << ",";
     O << formatDec((Imm & 0xc)  >> 2) << ",";

llvm/trunk/lib/Target/AMDGPU/SIDefines.h

enum ClassFlags {
     N_ZERO = 1 << 5,       // Negative zero
     P_ZERO = 1 << 6,       // Positive zero
     P_SUBNORMAL = 1 << 7,  // Positive subnormal
     P_NORMAL = 1 << 8,     // Positive normal
     P_INFINITY = 1 << 9    // Positive infinity
   };
 }
 
+// Input operand modifiers bit-masks
+// NEG and SEXT share same bit-mask because they can't be set simultaneously.
 namespace SISrcMods {
   enum {
-   NEG = 1 << 0,
-   ABS = 1 << 1
+   NEG = 1 << 0,  // Floating-point negate modifier
+   ABS = 1 << 1,  // Floating-point absolute modifier
+   SEXT = 1 << 0  // Integer sign-extend modifier
   };
 }
 
 namespace SIOutMods {
   enum {
     NONE = 0,
     MUL2 = 1,
     MUL4 = 2,

llvm/trunk/lib/Target/AMDGPU/SIInstrInfo.td

 
 def hwreg : NamedOperandU16<"Hwreg", NamedMatchClass<"Hwreg", 0>>;
 
 } // End OperandType = "OPERAND_IMMEDIATE"
 
 
 def VOPDstS64 : VOPDstOperand <SReg_64>;
 
+def FPInputModsMatchClass : AsmOperandClass {
+  let Name = "RegOrImmWithFPInputMods";
+  let ParserMethod = "parseRegOrImmWithFPInputMods";
+  let PredicateMethod = "isRegOrImmWithInputMods";
+}
+
+def FPInputMods : Operand <i32> {
+  let PrintMethod = "printOperandAndFPInputMods";
+  let ParserMatchClass = FPInputModsMatchClass;
+}
+
+def IntInputModsMatchClass : AsmOperandClass {
+  let Name = "RegOrImmWithIntInputMods";
+  let ParserMethod = "parseRegOrImmWithIntInputMods";
+  let PredicateMethod = "isRegOrImmWithInputMods";
+}
+
+def IntInputMods: Operand <i32> {
+  let PrintMethod = "printOperandAndIntInputMods";
+  let ParserMatchClass = IntInputModsMatchClass;
+}
+
 //===----------------------------------------------------------------------===//
 // Complex patterns
 //===----------------------------------------------------------------------===//
 
 def DS1Addr1Offset : ComplexPattern<i32, 2, "SelectDS1Addr1Offset">;
 def DS64Bit4ByteAligned : ComplexPattern<i32, 3, "SelectDS64Bit4ByteAligned">;
 
 def MUBUFAddr32 : ComplexPattern<i64, 9, "SelectMUBUFAddr32">;
defm _SGPR : SMRD_SOFF_m <
     opName#" $sdst, $sbase, $soff", []
   >;
 }
 
 //===----------------------------------------------------------------------===//
 // Vector ALU classes
 //===----------------------------------------------------------------------===//
 
-// This must always be right before the operand being input modified.
-def InputMods : OperandWithDefaultOps <i32, (ops (i32 0))> {
-  let PrintMethod = "printOperandAndMods";
-}
-
-def InputModsMatchClass : AsmOperandClass {
-  let Name = "RegOrImmWithInputMods";
-  let ParserMethod = "parseRegOrImmWithInputMods";
-}
-
-def InputModsNoDefault : Operand <i32> {
-  let PrintMethod = "printOperandAndMods";
-  let ParserMatchClass = InputModsMatchClass;
-}
-
 class getNumSrcArgs<ValueType Src0, ValueType Src1, ValueType Src2> {
   int ret =
     !if (!eq(Src0.Value, untyped.Value),      0,
       !if (!eq(Src1.Value, untyped.Value),    1,   // VOP1
          !if (!eq(Src2.Value, untyped.Value), 2,   // VOP2
                                               3))); // VOP3
 }
 
class getIns64 <RegisterOperand Src0RC, RegisterOperand Src1RC,
   dag ret =
     !if (!eq(NumSrcArgs, 0),
       // VOP1 without input operands (V_NOP, V_CLREXCP)
       (ins),
       /* else */
     !if (!eq(NumSrcArgs, 1),
       !if (!eq(HasModifiers, 1),
         // VOP1 with modifiers
-        (ins InputModsNoDefault:$src0_modifiers, Src0RC:$src0,
+        (ins FPInputMods:$src0_modifiers, Src0RC:$src0,
              clampmod:$clamp, omod:$omod)
       /* else */,
         // VOP1 without modifiers
         (ins Src0RC:$src0)
       /* endif */ ),
     !if (!eq(NumSrcArgs, 2),
       !if (!eq(HasModifiers, 1),
         // VOP 2 with modifiers
-        (ins InputModsNoDefault:$src0_modifiers, Src0RC:$src0,
-             InputModsNoDefault:$src1_modifiers, Src1RC:$src1,
+        (ins FPInputMods:$src0_modifiers, Src0RC:$src0,
+             FPInputMods:$src1_modifiers, Src1RC:$src1,
              clampmod:$clamp, omod:$omod)
       /* else */,
         // VOP2 without modifiers
         (ins Src0RC:$src0, Src1RC:$src1)
       /* endif */ )
     /* NumSrcArgs == 3 */,
       !if (!eq(HasModifiers, 1),
         // VOP3 with modifiers
-        (ins InputModsNoDefault:$src0_modifiers, Src0RC:$src0,
-             InputModsNoDefault:$src1_modifiers, Src1RC:$src1,
-             InputModsNoDefault:$src2_modifiers, Src2RC:$src2,
+        (ins FPInputMods:$src0_modifiers, Src0RC:$src0,
+             FPInputMods:$src1_modifiers, Src1RC:$src1,
+             FPInputMods:$src2_modifiers, Src2RC:$src2,
              clampmod:$clamp, omod:$omod)
       /* else */,
         // VOP3 without modifiers
         (ins Src0RC:$src0, Src1RC:$src1, Src2RC:$src2)
       /* endif */ ))));
 }
 
 class getInsDPP <RegisterClass Src0RC, RegisterClass Src1RC, int NumSrcArgs,
                                                              bit HasModifiers> {
 
   dag ret = !if (!eq(NumSrcArgs, 0),
                 // VOP1 without input operands (V_NOP)
                 (ins dpp_ctrl:$dpp_ctrl, row_mask:$row_mask,
                      bank_mask:$bank_mask, bound_ctrl:$bound_ctrl),
             !if (!eq(NumSrcArgs, 1),
               !if (!eq(HasModifiers, 1),
                 // VOP1_DPP with modifiers
-                (ins InputModsNoDefault:$src0_modifiers, Src0RC:$src0,
+                (ins FPInputMods:$src0_modifiers, Src0RC:$src0,
                      dpp_ctrl:$dpp_ctrl, row_mask:$row_mask,
                      bank_mask:$bank_mask, bound_ctrl:$bound_ctrl)
               /* else */,
                 // VOP1_DPP without modifiers
                 (ins Src0RC:$src0, dpp_ctrl:$dpp_ctrl, row_mask:$row_mask,
                 bank_mask:$bank_mask, bound_ctrl:$bound_ctrl)
               /* endif */)
               /* NumSrcArgs == 2 */,
               !if (!eq(HasModifiers, 1),
                 // VOP2_DPP with modifiers
-                (ins InputModsNoDefault:$src0_modifiers, Src0RC:$src0,
-                      InputModsNoDefault:$src1_modifiers, Src1RC:$src1,
-                      dpp_ctrl:$dpp_ctrl, row_mask:$row_mask,
-                      bank_mask:$bank_mask, bound_ctrl:$bound_ctrl)
+                (ins FPInputMods:$src0_modifiers, Src0RC:$src0,
+                     FPInputMods:$src1_modifiers, Src1RC:$src1,
               /* else */,
                 // VOP2_DPP without modifiers
                 (ins Src0RC:$src0, Src1RC:$src1, dpp_ctrl:$dpp_ctrl,
                 row_mask:$row_mask, bank_mask:$bank_mask,
                 bound_ctrl:$bound_ctrl)
              /* endif */)));
 }
 
 class getInsSDWA <RegisterClass Src0RC, RegisterClass Src1RC, int NumSrcArgs,
-                                                             bit HasModifiers> {
+                  bit HasFloatModifiers> {
 
   dag ret = !if (!eq(NumSrcArgs, 0),
                 // VOP1 without input operands (V_NOP)
                 (ins),
             !if (!eq(NumSrcArgs, 1),
-              !if (!eq(HasModifiers, 1),
-                // VOP1_SDWA with modifiers
-                (ins InputModsNoDefault:$src0_fmodifiers, Src0RC:$src0,
+              !if (HasFloatModifiers,
+                // VOP1_SDWA with float modifiers
+                (ins FPInputMods:$src0_fmodifiers, Src0RC:$src0,
                      clampmod:$clamp, dst_sel:$dst_sel, dst_unused:$dst_unused,
                      src0_sel:$src0_sel)
               /* else */,
-                // VOP1_SDWA without modifiers
-                // FIXME: sext() modifier is not supported yet
-                (ins Src0RC:$src0, dst_sel:$dst_sel, dst_unused:$dst_unused,
+                // VOP1_SDWA with sext modifier
+                (ins IntInputMods:$src0_imodifiers, Src0RC:$src0,
+                     clampmod:$clamp, dst_sel:$dst_sel, dst_unused:$dst_unused,
                      src0_sel:$src0_sel)
               /* endif */)
               /* NumSrcArgs == 2 */,
-              !if (!eq(HasModifiers, 1),
-                // VOP2_SDWA with modifiers
-                (ins InputModsNoDefault:$src0_fmodifiers, Src0RC:$src0,
-                     InputModsNoDefault:$src1_fmodifiers, Src1RC:$src1,
+              !if (HasFloatModifiers,
+                // VOP2_SDWA with float modifiers
+                (ins FPInputMods:$src0_fmodifiers, Src0RC:$src0,
+                     FPInputMods:$src1_fmodifiers, Src1RC:$src1,
                      clampmod:$clamp, dst_sel:$dst_sel, dst_unused:$dst_unused,
                      src0_sel:$src0_sel, src1_sel:$src1_sel)
               /* else */,
-                // VOP2_DPP without modifiers
-                (ins Src0RC:$src0, Src1RC:$src1,
-                     dst_sel:$dst_sel, dst_unused:$dst_unused,
+                // VOP2_DPP with sext modifier
+                (ins IntInputMods:$src0_imodifiers, Src0RC:$src0,
+                     IntInputMods:$src1_imodifiers, Src1RC:$src1,
+                     clampmod:$clamp, dst_sel:$dst_sel, dst_unused:$dst_unused,
                      src0_sel:$src0_sel, src1_sel:$src1_sel)
              /* endif */)));
 }
 
 // Outs for DPP and SDWA
 class getOutsExt <bit HasDst, ValueType DstVT, RegisterOperand DstRCDPP> {
   dag ret = !if(HasDst,
                 !if(!eq(DstVT.Size, 1),
string src1 = !if(!eq(NumSrcArgs, 1), "",
                    !if(!eq(NumSrcArgs, 2), " $src1_modifiers",
                                            " $src1_modifiers,"));
   string args = !if(!eq(HasModifiers, 0),
                      getAsm32<0, NumSrcArgs, DstVT>.ret,
                      ", "#src0#src1);
   string ret = dst#args#" $dpp_ctrl$row_mask$bank_mask$bound_ctrl";
 }
 
-class getAsmSDWA <bit HasDst, int NumSrcArgs, bit HasModifiers, ValueType DstVT = i32> {
-  string dst = !if(HasDst,
+class getAsmSDWA <bit HasDst, int NumSrcArgs, bit HasFloatModifiers,
+                  ValueType DstVT = i32> {
+  string dst = !if(HasDst, 
                    !if(!eq(DstVT.Size, 1),
-                       "$sdst",
+                       "$sdst", // use $sdst for VOPC
                        "$vdst"),
-                    ""); // use $sdst for VOPC
-  string src0 = !if(!eq(NumSrcArgs, 1), "$src0_fmodifiers", "$src0_fmodifiers,");
-  string src1 = !if(!eq(NumSrcArgs, 1), "",
-                   !if(!eq(NumSrcArgs, 2), " $src1_fmodifiers",
-                                           " $src1_fmodifiers,"));
-  string args = !if(!eq(HasModifiers, 0),
-                     getAsm32<0, NumSrcArgs, DstVT>.ret,
-                     ", "#src0#src1#"$clamp");
+                    "");
+  string src0 = !if(HasFloatModifiers, "$src0_fmodifiers", "$src0_imodifiers");
+  string src1 = !if(HasFloatModifiers, "$src1_fmodifiers", "$src1_imodifiers");
+  string args = !if(!eq(NumSrcArgs, 0),
+                    "", 
+                    !if(!eq(NumSrcArgs, 1), 
+                        ", "#src0#"$clamp",
+                        ", "#src0#", "#src1#"$clamp"
+                     )
+                );
   string sdwa = !if(!eq(NumSrcArgs, 0),
                     "",
                     !if(!eq(NumSrcArgs, 1),
                         " $dst_sel $dst_unused $src0_sel",
                         " $dst_sel $dst_unused $src0_sel $src1_sel"
                     )
                 );
   string ret = dst#args#sdwa;

 class VOPC_Profile<ValueType vt0, ValueType vt1 = vt0> : VOPProfile <[i1, vt0, vt1, untyped]> {
   let Asm32 = "vcc, $src0, $src1";
   // The destination for 32-bit encoding is implicit.
   let HasDst32 = 0;
   let Outs64 = (outs DstRC:$sdst);
 }
 
 class VOPC_Class_Profile<ValueType vt> : VOPC_Profile<vt, i32> {
-  let Ins64 = (ins InputModsNoDefault:$src0_modifiers, Src0RC64:$src0, Src1RC64:$src1);
+  let Ins64 = (ins FPInputMods:$src0_modifiers, Src0RC64:$src0, Src1RC64:$src1);
   let Asm64 = "$sdst, $src0_modifiers, $src1";
 }
 
 def VOPC_I1_F32_F32 : VOPC_Profile<f32>;
 def VOPC_I1_F64_F64 : VOPC_Profile<f64>;
 def VOPC_I1_I32_I32 : VOPC_Profile<i32>;
 def VOPC_I1_I64_I64 : VOPC_Profile<i64>;
 
def VOP_MADMK : VOPProfile <[f32, f32, f32, f32]> {
   field dag Ins32 = (ins VCSrc_32:$src0, u32imm:$imm, VGPR_32:$src1);
   field string Asm32 = "$vdst, $src0, $imm, $src1";
   field bit HasExt = 0;
 }
 def VOP_MAC : VOPProfile <[f32, f32, f32, f32]> {
   let Ins32 = (ins Src0RC32:$src0, Src1RC32:$src1, VGPR_32:$src2);
   let Ins64 = getIns64<Src0RC64, Src1RC64, RegisterOperand<VGPR_32>, 3,
                              HasModifiers>.ret;
-  let InsDPP = (ins InputModsNoDefault:$src0_modifiers, Src0RC32:$src0,
-                    InputModsNoDefault:$src1_modifiers, Src1RC32:$src1,
+  let InsDPP = (ins FPInputMods:$src0_modifiers, Src0RC32:$src0,
+                    FPInputMods:$src1_modifiers, Src1RC32:$src1,
                     VGPR_32:$src2, // stub argument
                     dpp_ctrl:$dpp_ctrl, row_mask:$row_mask,
                     bank_mask:$bank_mask, bound_ctrl:$bound_ctrl);
-  let InsSDWA = (ins InputModsNoDefault:$src0_fmodifiers, Src0RC32:$src0,
-                     InputModsNoDefault:$src1_fmodifiers, Src1RC32:$src1,
+  let InsSDWA = (ins FPInputMods:$src0_fmodifiers, Src0RC32:$src0,
+                     FPInputMods:$src1_fmodifiers, Src1RC32:$src1,
                      VGPR_32:$src2, // stub argument
                      clampmod:$clamp, dst_sel:$dst_sel, dst_unused:$dst_unused,
                      src0_sel:$src0_sel, src1_sel:$src1_sel);
   let Asm32 = getAsm32<1, 2, f32>.ret;
   let Asm64 = getAsm64<1, 2, HasModifiers, f32>.ret;
   let AsmDPP = getAsmDPP<1, 2, HasModifiers, f32>.ret;
   let AsmSDWA = getAsmSDWA<1, 2, HasModifiers, f32>.ret;
 }
class VOP1_DPP <vop1 op, string opName, VOPProfile p> :
   let DisableDecoder = DisableVIDecoder;
   let src0_modifiers = !if(p.HasModifiers, ?, 0);
   let src1_modifiers = 0;
 }
 
 class SDWADisableFields <VOPProfile p> {
   bits<8> src0 = !if(!eq(p.NumSrcArgs, 0), 0, ?);
   bits<3> src0_sel = !if(!eq(p.NumSrcArgs, 0), 6, ?);
-  bits<2> src0_fmodifiers = !if(p.HasModifiers, ?, 0);
-  bits<1> src0_imodifiers = 0; // FIXME: always 0 untill sext modifier is supported
+  bits<2> src0_fmodifiers = !if(!eq(p.NumSrcArgs, 0),
+                                0,
+                                !if(p.HasModifiers, ?, 0));
+  bits<1> src0_imodifiers = !if(!eq(p.NumSrcArgs, 0),
+                                0,
+                                !if(p.HasModifiers, 0, ?));
   bits<3> src1_sel = !if(!eq(p.NumSrcArgs, 0), 6,
                          !if(!eq(p.NumSrcArgs, 1), 6,
                              ?));
   bits<2> src1_fmodifiers = !if(!eq(p.NumSrcArgs, 0), 0,
                                 !if(!eq(p.NumSrcArgs, 1), 0,
                                     !if(p.HasModifiers, ?, 0)));
-  bits<1> src1_imodifiers = 0;
+  bits<1> src1_imodifiers = !if(!eq(p.NumSrcArgs, 0), 0,
+                                !if(!eq(p.NumSrcArgs, 1), 0,
+                                    !if(p.HasModifiers, 0, ?)));
   bits<3> dst_sel = !if(p.HasDst, ?, 6);
   bits<2> dst_unused = !if(p.HasDst, ?, 2);
-  bits<1> clamp = !if(p.HasModifiers, ?, 0);
+  bits<1> clamp = !if(!eq(p.NumSrcArgs, 0), 0, ?);
 }
 
 class VOP1_SDWA <vop1 op, string opName, VOPProfile p> :
   VOP1_SDWAe <op.VI>,
   VOP_SDWA <p.OutsSDWA, p.InsSDWA, opName#p.AsmSDWA, [], p.HasModifiers>,
   SDWADisableFields <p> {
-  let AsmMatchConverter = !if(!eq(p.HasModifiers,1), "cvtSdwaVop1_mod", "cvtSdwaVop1_nomod");
+  let AsmMatchConverter = "cvtSdwaVOP1";
   let AssemblerPredicates = !if(p.HasExt, [isVI], [DisableInst]);
   let DecoderNamespace = "SDWA";
   let DisableDecoder = DisableVIDecoder;
 }
 
 multiclass VOP1SI_m <vop1 op, string opName, VOPProfile p, list<dag> pattern,
                      string asm = opName#p.Asm32> {
 
class VOP2_DPP <vop2 op, string opName, VOPProfile p> :
   let src0_modifiers = !if(p.HasModifiers, ?, 0);
   let src1_modifiers = !if(p.HasModifiers, ?, 0);
 }
 
 class VOP2_SDWA <vop2 op, string opName, VOPProfile p> :
   VOP2_SDWAe <op.VI>,
   VOP_SDWA <p.OutsSDWA, p.InsSDWA, opName#p.AsmSDWA, [], p.HasModifiers>,
   SDWADisableFields <p> {
-  let AsmMatchConverter = !if(!eq(p.HasModifiers,1), "cvtSdwaVop2_mod", "cvtSdwaVop2_nomod");
+  let AsmMatchConverter = "cvtSdwaVOP2";
   let AssemblerPredicates = !if(p.HasExt, [isVI], [DisableInst]);
   let DecoderNamespace = "SDWA";
   let DisableDecoder = DisableVIDecoder;
 }
 
 class VOP3DisableFields <bit HasSrc1, bit HasSrc2, bit HasModifiers> {
 
   bits<2> src0_modifiers = !if(HasModifiers, ?, 0);

 
 // Special case for v_div_fmas_{f32|f64}, since it seems to be the
 // only VOP instruction that implicitly reads VCC.
 multiclass VOP3_VCC_Inst <vop3 op, string opName,
                           VOPProfile P,
                           SDPatternOperator node = null_frag> : VOP3_Helper <
   op, opName,
   (outs P.DstRC.RegClass:$vdst),
-  (ins InputModsNoDefault:$src0_modifiers, P.Src0RC64:$src0,
-       InputModsNoDefault:$src1_modifiers, P.Src1RC64:$src1,
-       InputModsNoDefault:$src2_modifiers, P.Src2RC64:$src2,
+  (ins FPInputMods:$src0_modifiers, P.Src0RC64:$src0,
+       FPInputMods:$src1_modifiers, P.Src1RC64:$src1,
+       FPInputMods:$src2_modifiers, P.Src2RC64:$src2,
        clampmod:$clamp,
        omod:$omod),
   "$vdst, $src0_modifiers, $src1_modifiers, $src2_modifiers"#"$clamp"#"$omod",
   [(set P.DstVT:$vdst,
             (node (P.Src0VT (VOP3Mods0 P.Src0VT:$src0, i32:$src0_modifiers,
                                        i1:$clamp, i32:$omod)),
                   (P.Src1VT (VOP3Mods P.Src1VT:$src1, i32:$src1_modifiers)),
                   (P.Src2VT (VOP3Mods P.Src2VT:$src2, i32:$src2_modifiers)),

llvm/trunk/test/MC/AMDGPU/vop_sdwa.s

 // NOSICI: error:
 // VI: v_min_u32_sdwa v1, v1, v1 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:DWORD ; encoding: [0xf9,0x02,0x02,0x1c,0x01,0x06,0x00,0x06]
 v_min_u32 v1, v1, v1 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:DWORD
 //---------------------------------------------------------------------------//
 // Check optional operands
 //---------------------------------------------------------------------------//
 
 // NOSICI: error:
-// VI: v_cvt_u32_f32_sdwa v0, v0 dst_sel:DWORD dst_unused:UNUSED_PRESERVE src0_sel:DWORD ; encoding: [0xf9,0x0e,0x00,0x7e,0x00,0x16,0x06,0x06]
-v_cvt_u32_f32 v0, v0 dst_sel:DWORD
+// VI: v_cvt_u32_f32_sdwa v0, v0 clamp dst_sel:DWORD dst_unused:UNUSED_PRESERVE src0_sel:DWORD ; encoding: [0xf9,0x0e,0x00,0x7e,0x00,0x36,0x06,0x06]
+v_cvt_u32_f32 v0, v0 clamp dst_sel:DWORD
 
 // NOSICI: error:
 // VI: v_fract_f32_sdwa v0, v0 clamp dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:DWORD ; encoding: [0xf9,0x36,0x00,0x7e,0x00,0x26,0x06,0x06]
 v_fract_f32 v0, v0 clamp dst_sel:DWORD dst_unused:UNUSED_PAD
 
 // NOSICI: error:
 // VI: v_sin_f32_sdwa v0, v0 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:WORD_1 ; encoding: [0xf9,0x52,0x00,0x7e,0x00,0x06,0x05,0x06]
 v_sin_f32 v0, v0 dst_unused:UNUSED_PAD src0_sel:WORD_1
 
 // NOSICI: error:
-// VI: v_mov_b32_sdwa v1, v0 dst_sel:DWORD dst_unused:UNUSED_PRESERVE src0_sel:WORD_1 ; encoding: [0xf9,0x02,0x02,0x7e,0x00,0x16,0x05,0x06]
-v_mov_b32 v1, v0 src0_sel:WORD_1
+// VI: v_mov_b32_sdwa v1, v0 clamp dst_sel:DWORD dst_unused:UNUSED_PRESERVE src0_sel:WORD_1 ; encoding: [0xf9,0x02,0x02,0x7e,0x00,0x36,0x05,0x06]
+v_mov_b32 v1, v0 clamp src0_sel:WORD_1
 
 // NOSICI: error:
 // VI: v_trunc_f32_sdwa v1, v0 clamp dst_sel:DWORD dst_unused:UNUSED_PRESERVE src0_sel:WORD_1 ; encoding: [0xf9,0x38,0x02,0x7e,0x00,0x36,0x05,0x06]
 v_trunc_f32 v1, v0 clamp dst_sel:DWORD src0_sel:WORD_1
 
 // NOSICI: error:
 // VI: v_mov_b32_sdwa v1, v0 dst_sel:DWORD dst_unused:UNUSED_PRESERVE src0_sel:DWORD ; encoding: [0xf9,0x02,0x02,0x7e,0x00,0x16,0x06,0x06]
 v_mov_b32_sdwa v1, v0
 
 // NOSICI: error:
 // VI: v_add_f32_sdwa v0, v0, v0 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:WORD_1 src1_sel:DWORD ; encoding: [0xf9,0x00,0x00,0x02,0x00,0x06,0x05,0x06]
 v_add_f32_sdwa v0, v0, v0 dst_unused:UNUSED_PAD src0_sel:WORD_1
 
 // NOSICI: error:
 // VI: v_min_f32_sdwa v0, v0, v0 clamp dst_sel:DWORD dst_unused:UNUSED_PRESERVE src0_sel:DWORD src1_sel:BYTE_2 ; encoding: [0xf9,0x00,0x00,0x14,0x00,0x36,0x06,0x02]
 v_min_f32 v0, v0, v0 clamp dst_sel:DWORD src1_sel:BYTE_2
 
 // NOSICI: error:
 // VI: v_and_b32_sdwa v0, v0, v0 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:DWORD src1_sel:BYTE_2 ; encoding: [0xf9,0x00,0x00,0x26,0x00,0x06,0x06,0x02]
 v_and_b32 v0, v0, v0 dst_unused:UNUSED_PAD src1_sel:BYTE_2
 
 // NOSICI: error:
-// VI: v_mul_i32_i24_sdwa v1, v2, v3 dst_sel:DWORD dst_unused:UNUSED_PRESERVE src0_sel:DWORD src1_sel:DWORD ; encoding: [0xf9,0x06,0x02,0x0c,0x02,0x16,0x06,0x06]
-v_mul_i32_i24_sdwa v1, v2, v3
+// VI: v_mul_i32_i24_sdwa v1, v2, v3 clamp dst_sel:DWORD dst_unused:UNUSED_PRESERVE src0_sel:DWORD src1_sel:DWORD ; encoding: [0xf9,0x06,0x02,0x0c,0x02,0x36,0x06,0x06]
+v_mul_i32_i24_sdwa v1, v2, v3 clamp
 
 //===----------------------------------------------------------------------===//
 // Check modifiers
 //===----------------------------------------------------------------------===//
 
 // NOSICI: error:
 // VI: v_fract_f32_sdwa v0, |v0| dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:WORD_1 ; encoding: [0xf9,0x36,0x00,0x7e,0x00,0x06,0x25,0x06]
 v_fract_f32 v0, |v0| dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:WORD_1
 
 // NOSICI: error:
 // VI: v_sin_f32_sdwa v0, -|v0| dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:WORD_1 ; encoding: [0xf9,0x52,0x00,0x7e,0x00,0x06,0x35,0x06]
 v_sin_f32 v0, -abs(v0) dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:WORD_1
 
 // NOSICI: error:
 // VI: v_add_f32_sdwa v0, -|v0|, -v0 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:WORD_1 src1_sel:BYTE_2 ; encoding: [0xf9,0x00,0x00,0x02,0x00,0x06,0x35,0x12]
 v_add_f32 v0, -|v0|, -v0 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:WORD_1 src1_sel:BYTE_2
 
 // NOSICI: error:
 // VI: v_min_f32_sdwa v0, |v0|, -v0 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:WORD_1 src1_sel:BYTE_2 ; encoding: [0xf9,0x00,0x00,0x14,0x00,0x06,0x25,0x12]
 v_min_f32 v0, abs(v0), -v0 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:WORD_1 src1_sel:BYTE_2
 
+// NOSICI: error:
+// VI: v_mov_b32_sdwa v1, sext(v0) dst_sel:DWORD dst_unused:UNUSED_PRESERVE src0_sel:DWORD ; encoding: [0xf9,0x02,0x02,0x7e,0x00,0x16,0x0e,0x06]
+v_mov_b32_sdwa v1, sext(v0)
+
+// NOSICI: error:
+// VI: v_and_b32_sdwa v0, sext(v0), sext(v0) dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:DWORD src1_sel:BYTE_2 ; encoding: [0xf9,0x00,0x00,0x26,0x00,0x06,0x0e,0x0a]
+v_and_b32 v0, sext(v0), sext(v0) dst_unused:UNUSED_PAD src1_sel:BYTE_2
+
 //===----------------------------------------------------------------------===//
 // Check VOP1 opcodes
 //===----------------------------------------------------------------------===//
 
 // NOSICI: error:
 // VI: v_nop ; encoding: [0xf9,0x00,0x00,0x7e,0x00,0x16,0x06,0x06]
 v_nop_sdwa