[llvm-commits] [llvm] r173572 - X86: Decode PALIGN operands so I don't have to do it in my head.

Pete Cooper peter_cooper at apple.com
Mon Apr 20 10:35:52 PDT 2015


> On Apr 18, 2015, at 5:32 PM, Craig Topper <craig.topper at gmail.com> wrote:
> 
> X86ShuffleDecode.cpp is also importing Constant.h from the IR layer.
Hmm, yeah.  Thanks Craig, i’ll add that to the list.  And thanks for already fixing an issue in r235262.

So the layering issues we have so far are:
- X86ShuffleDecode.cpp importing IR
- X86ShuffleDecode.cpp importing CodeGen
- ASAN asm instrumentation imports CodeGen
- libObject (used by llvm-mc) imports IR because of IROjectFile

I’ll see what i can fix.  Patches incoming soon :)

Cheers,
Pete
> 
> On Sat, Apr 18, 2015 at 12:42 PM, Pete Cooper <peter_cooper at apple.com <mailto:peter_cooper at apple.com>> wrote:
> 
> 
> Sent from my iPhone
> 
> > On Apr 18, 2015, at 10:04 AM, Benjamin Kramer <benny.kra at gmail.com <mailto:benny.kra at gmail.com>> wrote:
> >
> >
> >> On 18.04.2015, at 01:44, Pete Cooper <peter_cooper at apple.com <mailto:peter_cooper at apple.com>> wrote:
> >>
> >> Hi Benjamin
> >>
> >> Sorry to resurrect an old thread, however, this commit introduced (or perhaps extended) a bad layering violation to the X86 backend.
> >>
> >> Specifically, you’ve caused the MC layer InstPrinter to use MVT and X86ShuffleDecode.cpp which is a CodeGen level pass.
> >>
> >> Can you please find a way to refactored this to avoid the layering issue.
> >
> > The issue has been there long before this specific commit, dating back to at least 2011.
> Didn't dig that far back. Thanks for the svn archaeology.
> > Fixing it requires quite a bit of interface change as MVT is everywhere in X86InstComments, the only proper way of fixing this is passing the element size and vector size manually into every function. Feel free to make that change
> An alternative is to sink MVT to MC and keep EVT at the codegen level. I'm happy with either approach (and I'm happy to do the work). What do you think?
> 
> Cheers
> Pete
> >
> > - Ben
> >
> >>> On Jan 26, 2013, at 5:31 AM, Benjamin Kramer <benny.kra at googlemail.com <mailto:benny.kra at googlemail.com>> wrote:
> >>>
> >>> Author: d0k
> >>> Date: Sat Jan 26 07:31:37 2013
> >>> New Revision: 173572
> >>>
> >>> URL: http://llvm.org/viewvc/llvm-project?rev=173572&view=rev <http://llvm.org/viewvc/llvm-project?rev=173572&view=rev>
> >>> Log:
> >>> X86: Decode PALIGN operands so I don't have to do it in my head.
> >>>
> >>> Added:
> >>>  llvm/trunk/test/MC/X86/shuffle-comments.s
> >>> Modified:
> >>>  llvm/trunk/lib/Target/X86/InstPrinter/X86InstComments.cpp
> >>>  llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp
> >>>  llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h
> >>>  llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> >>>
> >>> Modified: llvm/trunk/lib/Target/X86/InstPrinter/X86InstComments.cpp
> >>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/InstPrinter/X86InstComments.cpp?rev=173572&r1=173571&r2=173572&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/InstPrinter/X86InstComments.cpp?rev=173572&r1=173571&r2=173572&view=diff>
> >>> ==============================================================================
> >>> --- llvm/trunk/lib/Target/X86/InstPrinter/X86InstComments.cpp (original)
> >>> +++ llvm/trunk/lib/Target/X86/InstPrinter/X86InstComments.cpp Sat Jan 26 07:31:37 2013
> >>> @@ -69,6 +69,28 @@ void llvm::EmitAnyX86InstComments(const
> >>>   DecodeMOVHLPSMask(2, ShuffleMask);
> >>>   break;
> >>>
> >>> +  case X86::PALIGNR128rr:
> >>> +  case X86::VPALIGNR128rr:
> >>> +    Src1Name = getRegName(MI->getOperand(2).getReg());
> >>> +    // FALL THROUGH.
> >>> +  case X86::PALIGNR128rm:
> >>> +  case X86::VPALIGNR128rm:
> >>> +    Src2Name = getRegName(MI->getOperand(1).getReg());
> >>> +    DestName = getRegName(MI->getOperand(0).getReg());
> >>> +    DecodePALIGNMask(MVT::v16i8,
> >>> +                     MI->getOperand(MI->getNumOperands()-1).getImm(),
> >>> +                     ShuffleMask);
> >>> +    break;
> >>> +  case X86::VPALIGNR256rr:
> >>> +    Src1Name = getRegName(MI->getOperand(2).getReg());
> >>> +    // FALL THROUGH.
> >>> +  case X86::VPALIGNR256rm:
> >>> +    Src2Name = getRegName(MI->getOperand(1).getReg());
> >>> +    DestName = getRegName(MI->getOperand(0).getReg());
> >>> +    DecodePALIGNMask(MVT::v32i8,
> >>> +                     MI->getOperand(MI->getNumOperands()-1).getImm(),
> >>> +                     ShuffleMask);
> >>> +
> >>> case X86::PSHUFDri:
> >>> case X86::VPSHUFDri:
> >>>   Src1Name = getRegName(MI->getOperand(1).getReg());
> >>>
> >>> 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=173572&r1=173571&r2=173572&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp?rev=173572&r1=173571&r2=173572&view=diff>
> >>> ==============================================================================
> >>> --- llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp (original)
> >>> +++ llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp Sat Jan 26 07:31:37 2013
> >>> @@ -61,6 +61,14 @@ void DecodeMOVLHPSMask(unsigned NElts, S
> >>>   ShuffleMask.push_back(NElts+i);
> >>> }
> >>>
> >>> +void DecodePALIGNMask(MVT VT, unsigned Imm, SmallVectorImpl<int> &ShuffleMask) {
> >>> +  unsigned NumElts = VT.getVectorNumElements();
> >>> +  unsigned Offset = Imm * (VT.getVectorElementType().getSizeInBits() / 8);
> >>> +
> >>> +  for (unsigned i = 0; i != NumElts; ++i)
> >>> +    ShuffleMask.push_back((i + Offset) % (NumElts * 2));
> >>> +}
> >>> +
> >>> /// DecodePSHUFMask - This decodes the shuffle masks for pshufd, and vpermilp*.
> >>> /// VT indicates the type of the vector allowing it to handle different
> >>> /// datatypes and vector widths.
> >>>
> >>> 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=173572&r1=173571&r2=173572&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h?rev=173572&r1=173571&r2=173572&view=diff>
> >>> ==============================================================================
> >>> --- llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h (original)
> >>> +++ llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h Sat Jan 26 07:31:37 2013
> >>> @@ -35,6 +35,8 @@ void DecodeMOVHLPSMask(unsigned NElts, S
> >>> // <0,2> or <0,1,4,5>
> >>> void DecodeMOVLHPSMask(unsigned NElts, SmallVectorImpl<int> &ShuffleMask);
> >>>
> >>> +void DecodePALIGNMask(MVT VT, unsigned Imm, SmallVectorImpl<int> &ShuffleMask);
> >>> +
> >>> void DecodePSHUFMask(MVT VT, unsigned Imm, SmallVectorImpl<int> &ShuffleMask);
> >>>
> >>> void DecodePSHUFHWMask(MVT VT, unsigned Imm, SmallVectorImpl<int> &ShuffleMask);
> >>>
> >>> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> >>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=173572&r1=173571&r2=173572&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=173572&r1=173571&r2=173572&view=diff>
> >>> ==============================================================================
> >>> --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
> >>> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Sat Jan 26 07:31:37 2013
> >>> @@ -4592,6 +4592,10 @@ static bool getTargetShuffleMask(SDNode
> >>> case X86ISD::MOVLHPS:
> >>>   DecodeMOVLHPSMask(NumElems, Mask);
> >>>   break;
> >>> +  case X86ISD::PALIGN:
> >>> +    ImmN = N->getOperand(N->getNumOperands()-1);
> >>> +    DecodePALIGNMask(VT, cast<ConstantSDNode>(ImmN)->getZExtValue(), Mask);
> >>> +    break;
> >>> case X86ISD::PSHUFD:
> >>> case X86ISD::VPERMILP:
> >>>   ImmN = N->getOperand(N->getNumOperands()-1);
> >>> @@ -4635,7 +4639,6 @@ static bool getTargetShuffleMask(SDNode
> >>> case X86ISD::MOVLPS:
> >>> case X86ISD::MOVSHDUP:
> >>> case X86ISD::MOVSLDUP:
> >>> -  case X86ISD::PALIGN:
> >>>   // Not yet implemented
> >>>   return false;
> >>> default: llvm_unreachable("unknown target shuffle node");
> >>>
> >>> Added: llvm/trunk/test/MC/X86/shuffle-comments.s
> >>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/shuffle-comments.s?rev=173572&view=auto <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/shuffle-comments.s?rev=173572&view=auto>
> >>> ==============================================================================
> >>> --- llvm/trunk/test/MC/X86/shuffle-comments.s (added)
> >>> +++ llvm/trunk/test/MC/X86/shuffle-comments.s Sat Jan 26 07:31:37 2013
> >>> @@ -0,0 +1,31 @@
> >>> +# RUN: llvm-mc %s -triple=x86_64-unknown-unknown | FileCheck %s
> >>> +
> >>> +palignr $8, %xmm0, %xmm1
> >>> +# CHECK: xmm1 = xmm0[8,9,10,11,12,13,14,15],xmm1[0,1,2,3,4,5,6,7]
> >>> +palignr $8, (%rax), %xmm1
> >>> +# CHECK: xmm1 = mem[8,9,10,11,12,13,14,15],xmm1[0,1,2,3,4,5,6,7]
> >>> +
> >>> +palignr $16, %xmm0, %xmm1
> >>> +# CHECK: xmm1 = xmm1[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]
> >>> +palignr $16, (%rax), %xmm1
> >>> +# CHECK: xmm1 = xmm1[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]
> >>> +
> >>> +palignr $0, %xmm0, %xmm1
> >>> +# CHECK: xmm1 = xmm0[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]
> >>> +palignr $0, (%rax), %xmm1
> >>> +# CHECK: xmm1 = mem[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]
> >>> +
> >>> +vpalignr $8, %xmm0, %xmm1, %xmm2
> >>> +# CHECK: xmm2 = xmm0[8,9,10,11,12,13,14,15],xmm1[0,1,2,3,4,5,6,7]
> >>> +vpalignr $8, (%rax), %xmm1, %xmm2
> >>> +# CHECK: xmm2 = mem[8,9,10,11,12,13,14,15],xmm1[0,1,2,3,4,5,6,7]
> >>> +
> >>> +vpalignr $16, %xmm0, %xmm1, %xmm2
> >>> +# CHECK: xmm2 = xmm1[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]
> >>> +vpalignr $16, (%rax), %xmm1, %xmm2
> >>> +# CHECK: xmm2 = xmm1[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]
> >>> +
> >>> +vpalignr $0, %xmm0, %xmm1, %xmm2
> >>> +# CHECK: xmm2 = xmm0[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]
> >>> +vpalignr $0, (%rax), %xmm1, %xmm2
> >>> +# CHECK: xmm2 = mem[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]
> >>>
> >>>
> >>> _______________________________________________
> >>> 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>
> 
> 
> 
> -- 
> ~Craig

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


More information about the llvm-commits mailing list