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

Pete Cooper peter_cooper at apple.com
Thu May 28 16:50:43 PDT 2015


> On May 28, 2015, at 4:48 PM, Chandler Carruth <chandlerc at gmail.com> wrote:
> 
> I just hadn't seen this email in eons…
No worries, thanks for the reply.
> 
> I have absolutely no ideas about how to fix this.... yuck.
Best I can think of is split Utils in to CodeGenUtils and MCUtils.  Means splitting the files too though.  Or maybe just move the IR specific stuff in to the same library of even file that is using it.
> 
> I'll have to spend some time poking around at things to even make heads or tails of it.
Sounds good.  Thanks.

Pete
> 
> On Thu, May 28, 2015 at 4:32 PM Pete Cooper <peter_cooper at apple.com <mailto:peter_cooper at apple.com>> wrote:
> Ping.
> 
> Chandler, this is a pretty bad layering violation.  I’d really like to come up with a good solution for fixing it.
> 
> And i know you’re interested in this kind of thing because you said so yesterday ;) (http://lists.cs.uiuc.edu/pipermail/llvmdev/2015-May/086127.html <http://lists.cs.uiuc.edu/pipermail/llvmdev/2015-May/086127.html>)
> 
> Cheers,
> Pete
> 
>> On May 14, 2015, at 11:03 AM, Pete Cooper <peter_cooper at apple.com <mailto:peter_cooper at apple.com>> wrote:
>> 
>> Ping
>>> On Apr 20, 2015, at 3:18 PM, Pete Cooper <peter_cooper at apple.com <mailto:peter_cooper at apple.com>> wrote:
>>> 
>>> Hi Chandler
>>> 
>>> Sorry to resurrect an old thread.
>>> 
>>> X86ShuffleDecode.h is great in that it keeps all of shuffle decoding in a single place.  However, as its included by X86InstComments.cpp which is an MC level file, we now have libCore linked in to llvm-mc.
>>> 
>>> Although it breaks the ‘nice to have it all in one place comment i just made’, i can’t think of any other solution than to split this file in to CodeGen shuffle decoding and MC shuffle decoding.  Perhaps having the CodeGen one include the MC one to avoid #include churn.  That would also involve actually breaking the X86Utils library in to probably X86CodeGenUtils and X86MCUtils.
>>> 
>>> Can you think of any better solutions?  I’m happy to do the work if we can come up with the right answer.
>>> 
>>> Cheers,
>>> Pete
>>>> On Jul 25, 2014, at 4:47 PM, Chandler Carruth <chandlerc at gmail.com <mailto: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 <https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D213986-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=xbH8d62RqOQiXpUYSInKnixKOUOfShA0wLU6KZi9Oqo&s=WY407Y5kviiZwPCBkvjPwEqvfwXAA2Y10FTqieFr1WI&e=>
>>>> 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 <https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Target_X86_Utils_X86ShuffleDecode.cpp-3Frev-3D213986-26r1-3D213985-26r2-3D213986-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=xbH8d62RqOQiXpUYSInKnixKOUOfShA0wLU6KZi9Oqo&s=iOwtSUWjUS67609x31Uow8Q2q6uTaiP_msZc1HY5q50&e=>
>>>> ==============================================================================
>>>> --- 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 <https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Target_X86_Utils_X86ShuffleDecode.h-3Frev-3D213986-26r1-3D213985-26r2-3D213986-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=xbH8d62RqOQiXpUYSInKnixKOUOfShA0wLU6KZi9Oqo&s=5KqIXKoWWdQuZLWTUp-BzIoZaWXrLudmz7CArUKN9Ks&e=>
>>>> ==============================================================================
>>>> --- 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 <https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Target_X86_X86MCInstLower.cpp-3Frev-3D213986-26r1-3D213985-26r2-3D213986-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=xbH8d62RqOQiXpUYSInKnixKOUOfShA0wLU6KZi9Oqo&s=E2-8mrMsXL7uzW0xWGu_cGPq5D7K5voXmUM2p_2R_cw&e=>
>>>> ==============================================================================
>>>> --- 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 <mailto:llvm-commits at cs.uiuc.edu>
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>>> 
>> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150528/6deb7bae/attachment.html>


More information about the llvm-commits mailing list