[llvm] r213986 - [x86] Teach the X86 backend to print shuffle comments for PSHUFB

Pete Cooper peter_cooper at apple.com
Fri Jul 25 17:22:09 PDT 2014


Hi Chandler

You’re doing a decent amount of work here which will get thrown away by the MCAsmStreamer unless IsVerboseAsm is on, and its a text stream.

Would it be possible to check for the presence of the comment stream before doing any of the work here?

Note, actually most of the code in EmitInstruction should be checking for the comment stream, but one step at a time I guess :)

Thanks,
Pete
On Jul 25, 2014, at 4:47 PM, Chandler Carruth <chandlerc at gmail.com> wrote:

> Author: chandlerc
> Date: Fri Jul 25 18:47:11 2014
> New Revision: 213986
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=213986&view=rev
> Log:
> [x86] Teach the X86 backend to print shuffle comments for PSHUFB
> instructions which happen to have a constant mask.
> 
> Currently, this only handles a very narrow set of cases, but those
> happen to be the cases that I care about for testing shuffles sanely.
> This is a bit trickier than other shuffle instructions because we're
> decoding constants out of the constant pool. The current MC layer makes
> it completely impossible to inspect a constant pool entry, so we have to
> do it at the MI level and attach the comment to the streamer on its way
> out. So no joy for disassembling, but it does make test cases and asm
> dumps *much* nicer.
> 
> Sorry for no test cases, but it didn't really seem that valuable to go
> trolling through existing old test cases and updating them. I'll have
> lots of testing of this in the upcoming patch for SSSE3 emission in the
> new vector shuffle lowering code paths.
> 
> Modified:
>    llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp
>    llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h
>    llvm/trunk/lib/Target/X86/X86MCInstLower.cpp
> 
> Modified: llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp?rev=213986&r1=213985&r2=213986&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp (original)
> +++ llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp Fri Jul 25 18:47:11 2014
> @@ -13,6 +13,7 @@
> //===----------------------------------------------------------------------===//
> 
> #include "X86ShuffleDecode.h"
> +#include "llvm/IR/Constants.h"
> #include "llvm/CodeGen/MachineValueType.h"
> 
> //===----------------------------------------------------------------------===//
> @@ -207,6 +208,38 @@ void DecodeVPERM2X128Mask(MVT VT, unsign
>   }
> }
> 
> +/// \brief Decode PSHUFB masks stored in an LLVM Constant.
> +void DecodePSHUFBMask(const ConstantDataSequential *C,
> +                      SmallVectorImpl<int> &ShuffleMask) {
> +  Type *MaskTy = C->getType();
> +  assert(MaskTy->isVectorTy() && "Expected a vector constant mask!");
> +  Type *EltTy = MaskTy->getVectorElementType();
> +  assert(EltTy->isIntegerTy(8) && "Expected i8 constant mask elements!");
> +  int NumElements = MaskTy->getVectorNumElements();
> +  // FIXME: Add support for AVX-512.
> +  assert((NumElements == 16 || NumElements == 32) &&
> +         "Only 128-bit and 256-bit vectors supported!");
> +  assert((unsigned)NumElements == C->getNumElements() &&
> +         "Constant mask has a different number of elements!");
> +
> +  ShuffleMask.reserve(NumElements);
> +  for (int i = 0; i < NumElements; ++i) {
> +    // For AVX vectors with 32 bytes the base of the shuffle is the half of the
> +    // vector we're inside.
> +    int Base = i < 16 ? 0 : 16;
> +    uint64_t Element = C->getElementAsInteger(i);
> +    // If the high bit (7) of the byte is set, the element is zeroed.
> +    if (Element & (1 << 7))
> +      ShuffleMask.push_back(SM_SentinelZero);
> +    else {
> +      int Index = Base + Element;
> +      assert((Index >= 0 && Index < NumElements) ||
> +             "Out of bounds shuffle index for pshub instruction!");
> +      ShuffleMask.push_back(Index);
> +    }
> +  }
> +}
> +
> /// DecodeVPERMMask - this decodes the shuffle masks for VPERMQ/VPERMPD.
> /// No VT provided since it only works on 256-bit, 4 element vectors.
> void DecodeVPERMMask(unsigned Imm, SmallVectorImpl<int> &ShuffleMask) {
> 
> Modified: llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h?rev=213986&r1=213985&r2=213986&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h (original)
> +++ llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h Fri Jul 25 18:47:11 2014
> @@ -22,6 +22,7 @@
> //===----------------------------------------------------------------------===//
> 
> namespace llvm {
> +class ConstantDataSequential;
> class MVT;
> 
> enum {
> @@ -59,6 +60,8 @@ void DecodeUNPCKHMask(MVT VT, SmallVecto
> /// different datatypes and vector widths.
> void DecodeUNPCKLMask(MVT VT, SmallVectorImpl<int> &ShuffleMask);
> 
> +void DecodePSHUFBMask(const ConstantDataSequential *C,
> +                      SmallVectorImpl<int> &ShuffleMask);
> 
> void DecodeVPERM2X128Mask(MVT VT, unsigned Imm,
>                           SmallVectorImpl<int> &ShuffleMask);
> 
> Modified: llvm/trunk/lib/Target/X86/X86MCInstLower.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86MCInstLower.cpp?rev=213986&r1=213985&r2=213986&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/X86MCInstLower.cpp (original)
> +++ llvm/trunk/lib/Target/X86/X86MCInstLower.cpp Fri Jul 25 18:47:11 2014
> @@ -16,8 +16,11 @@
> #include "X86RegisterInfo.h"
> #include "InstPrinter/X86ATTInstPrinter.h"
> #include "MCTargetDesc/X86BaseInfo.h"
> +#include "Utils/X86ShuffleDecode.h"
> #include "llvm/ADT/SmallString.h"
> #include "llvm/CodeGen/MachineFunction.h"
> +#include "llvm/CodeGen/MachineConstantPool.h"
> +#include "llvm/CodeGen/MachineOperand.h"
> #include "llvm/CodeGen/MachineModuleInfoImpls.h"
> #include "llvm/CodeGen/StackMaps.h"
> #include "llvm/IR/DataLayout.h"
> @@ -963,6 +966,83 @@ void X86AsmPrinter::EmitInstruction(cons
>   case X86::SEH_EndPrologue:
>     OutStreamer.EmitWinCFIEndProlog();
>     return;
> +
> +  case X86::PSHUFBrm:
> +    // Lower PSHUFB normally but add a comment if we can find a constant
> +    // shuffle mask. We won't be able to do this at the MC layer because the
> +    // mask isn't an immediate.
> +    std::string Comment;
> +    raw_string_ostream CS(Comment);
> +    SmallVector<int, 16> Mask;
> +
> +    assert(MI->getNumOperands() == 7 &&
> +           "Wrong number of operansd for PSHUFBrm");
> +    const MachineOperand &DstOp = MI->getOperand(0);
> +    const MachineOperand &SrcOp = MI->getOperand(1);
> +    const MachineOperand &MaskOp = MI->getOperand(5);
> +
> +    // Compute the name for a register. This is really goofy because we have
> +    // multiple instruction printers that could (in theory) use different
> +    // names. Fortunately most people use the ATT style (outside of Windows)
> +    // and they actually agree on register naming here. Ultimately, this is
> +    // a comment, and so its OK if it isn't perfect.
> +    auto GetRegisterName = [](unsigned RegNum) -> StringRef {
> +      return X86ATTInstPrinter::getRegisterName(RegNum);
> +    };
> +
> +    StringRef DstName = DstOp.isReg() ? GetRegisterName(DstOp.getReg()) : "mem";
> +    StringRef SrcName = SrcOp.isReg() ? GetRegisterName(SrcOp.getReg()) : "mem";
> +    CS << DstName << " = ";
> +
> +    if (MaskOp.isCPI()) {
> +      ArrayRef<MachineConstantPoolEntry> Constants =
> +          MI->getParent()->getParent()->getConstantPool()->getConstants();
> +      const MachineConstantPoolEntry &MaskConstantEntry =
> +          Constants[MI->getOperand(5).getIndex()];
> +      Type *MaskTy = MaskConstantEntry.getType();
> +      if (!MaskConstantEntry.isMachineConstantPoolEntry())
> +        if (auto *C = dyn_cast<ConstantDataSequential>(
> +                MaskConstantEntry.Val.ConstVal)) {
> +          assert(MaskTy == C->getType() &&
> +                 "Expected a constant of the same type!");
> +
> +          DecodePSHUFBMask(C, Mask);
> +          assert(Mask.size() == MaskTy->getVectorNumElements() &&
> +                 "Shuffle mask has a different size than its type!");
> +        }
> +    }
> +
> +    if (!Mask.empty()) {
> +      bool NeedComma = false;
> +      bool InSrc = false;
> +      for (int M : Mask) {
> +        // Wrap up any prior entry...
> +        if (M == SM_SentinelZero && InSrc) {
> +          InSrc = false;
> +          CS << "]";
> +        }
> +        if (NeedComma)
> +          CS << ",";
> +        else
> +          NeedComma = true;
> +
> +        // Print this shuffle...
> +        if (M == SM_SentinelZero) {
> +          CS << "zero";
> +        } else {
> +          if (!InSrc) {
> +            InSrc = true;
> +            CS << SrcName << "[";
> +          }
> +          CS << M;
> +        }
> +      }
> +      if (InSrc)
> +        CS << "]";
> +
> +      OutStreamer.AddComment(CS.str());
> +    }
> +    break;
>   }
> 
>   MCInst TmpInst;
> 
> 
> _______________________________________________
> 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