<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Apr 18, 2015, at 5:32 PM, Craig Topper <<a href="mailto:craig.topper@gmail.com" class="">craig.topper@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">X86ShuffleDecode.cpp is also importing Constant.h from the IR layer.</div></div></blockquote>Hmm, yeah. Thanks Craig, i’ll add that to the list. And thanks for already fixing an issue in r235262.</div><div><br class=""></div><div>So the layering issues we have so far are:</div><div>- X86ShuffleDecode.cpp importing IR</div><div>- X86ShuffleDecode.cpp importing CodeGen</div><div>- ASAN asm instrumentation imports CodeGen</div><div>- libObject (used by llvm-mc) imports IR because of IROjectFile</div><div><br class=""></div><div>I’ll see what i can fix. Patches incoming soon :)</div><div><br class=""></div><div>Cheers,</div><div>Pete<br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Sat, Apr 18, 2015 at 12:42 PM, Pete Cooper <span dir="ltr" class=""><<a href="mailto:peter_cooper@apple.com" target="_blank" class="">peter_cooper@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br class="">
<br class="">
Sent from my iPhone<br class="">
<span class=""><br class="">
> On Apr 18, 2015, at 10:04 AM, Benjamin Kramer <<a href="mailto:benny.kra@gmail.com" class="">benny.kra@gmail.com</a>> wrote:<br class="">
><br class="">
><br class="">
>> On 18.04.2015, at 01:44, Pete Cooper <<a href="mailto:peter_cooper@apple.com" class="">peter_cooper@apple.com</a>> wrote:<br class="">
>><br class="">
>> Hi Benjamin<br class="">
>><br class="">
>> Sorry to resurrect an old thread, however, this commit introduced (or perhaps extended) a bad layering violation to the X86 backend.<br class="">
>><br class="">
>> Specifically, you’ve caused the MC layer InstPrinter to use MVT and X86ShuffleDecode.cpp which is a CodeGen level pass.<br class="">
>><br class="">
>> Can you please find a way to refactored this to avoid the layering issue.<br class="">
><br class="">
> The issue has been there long before this specific commit, dating back to at least 2011.<br class="">
</span>Didn't dig that far back. Thanks for the svn archaeology.<br class="">
<span class="">> 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<br class="">
</span>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?<br class="">
<br class="">
Cheers<br class="">
<span class="HOEnZb"><font color="#888888" class="">Pete<br class="">
</font></span><div class="HOEnZb"><div class="h5">><br class="">
> - Ben<br class="">
><br class="">
>>> On Jan 26, 2013, at 5:31 AM, Benjamin Kramer <<a href="mailto:benny.kra@googlemail.com" class="">benny.kra@googlemail.com</a>> wrote:<br class="">
>>><br class="">
>>> Author: d0k<br class="">
>>> Date: Sat Jan 26 07:31:37 2013<br class="">
>>> New Revision: 173572<br class="">
>>><br class="">
>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=173572&view=rev" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=173572&view=rev</a><br class="">
>>> Log:<br class="">
>>> X86: Decode PALIGN operands so I don't have to do it in my head.<br class="">
>>><br class="">
>>> Added:<br class="">
>>> llvm/trunk/test/MC/X86/shuffle-comments.s<br class="">
>>> Modified:<br class="">
>>> llvm/trunk/lib/Target/X86/InstPrinter/X86InstComments.cpp<br class="">
>>> llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp<br class="">
>>> llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h<br class="">
>>> llvm/trunk/lib/Target/X86/X86ISelLowering.cpp<br class="">
>>><br class="">
>>> Modified: llvm/trunk/lib/Target/X86/InstPrinter/X86InstComments.cpp<br class="">
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/InstPrinter/X86InstComments.cpp?rev=173572&r1=173571&r2=173572&view=diff" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/InstPrinter/X86InstComments.cpp?rev=173572&r1=173571&r2=173572&view=diff</a><br class="">
>>> ==============================================================================<br class="">
>>> --- llvm/trunk/lib/Target/X86/InstPrinter/X86InstComments.cpp (original)<br class="">
>>> +++ llvm/trunk/lib/Target/X86/InstPrinter/X86InstComments.cpp Sat Jan 26 07:31:37 2013<br class="">
>>> @@ -69,6 +69,28 @@ void llvm::EmitAnyX86InstComments(const<br class="">
>>> DecodeMOVHLPSMask(2, ShuffleMask);<br class="">
>>> break;<br class="">
>>><br class="">
>>> + case X86::PALIGNR128rr:<br class="">
>>> + case X86::VPALIGNR128rr:<br class="">
>>> + Src1Name = getRegName(MI->getOperand(2).getReg());<br class="">
>>> + // FALL THROUGH.<br class="">
>>> + case X86::PALIGNR128rm:<br class="">
>>> + case X86::VPALIGNR128rm:<br class="">
>>> + Src2Name = getRegName(MI->getOperand(1).getReg());<br class="">
>>> + DestName = getRegName(MI->getOperand(0).getReg());<br class="">
>>> + DecodePALIGNMask(MVT::v16i8,<br class="">
>>> + MI->getOperand(MI->getNumOperands()-1).getImm(),<br class="">
>>> + ShuffleMask);<br class="">
>>> + break;<br class="">
>>> + case X86::VPALIGNR256rr:<br class="">
>>> + Src1Name = getRegName(MI->getOperand(2).getReg());<br class="">
>>> + // FALL THROUGH.<br class="">
>>> + case X86::VPALIGNR256rm:<br class="">
>>> + Src2Name = getRegName(MI->getOperand(1).getReg());<br class="">
>>> + DestName = getRegName(MI->getOperand(0).getReg());<br class="">
>>> + DecodePALIGNMask(MVT::v32i8,<br class="">
>>> + MI->getOperand(MI->getNumOperands()-1).getImm(),<br class="">
>>> + ShuffleMask);<br class="">
>>> +<br class="">
>>> case X86::PSHUFDri:<br class="">
>>> case X86::VPSHUFDri:<br class="">
>>> Src1Name = getRegName(MI->getOperand(1).getReg());<br class="">
>>><br class="">
>>> Modified: llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp<br class="">
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp?rev=173572&r1=173571&r2=173572&view=diff" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp?rev=173572&r1=173571&r2=173572&view=diff</a><br class="">
>>> ==============================================================================<br class="">
>>> --- llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp (original)<br class="">
>>> +++ llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp Sat Jan 26 07:31:37 2013<br class="">
>>> @@ -61,6 +61,14 @@ void DecodeMOVLHPSMask(unsigned NElts, S<br class="">
>>> ShuffleMask.push_back(NElts+i);<br class="">
>>> }<br class="">
>>><br class="">
>>> +void DecodePALIGNMask(MVT VT, unsigned Imm, SmallVectorImpl<int> &ShuffleMask) {<br class="">
>>> + unsigned NumElts = VT.getVectorNumElements();<br class="">
>>> + unsigned Offset = Imm * (VT.getVectorElementType().getSizeInBits() / 8);<br class="">
>>> +<br class="">
>>> + for (unsigned i = 0; i != NumElts; ++i)<br class="">
>>> + ShuffleMask.push_back((i + Offset) % (NumElts * 2));<br class="">
>>> +}<br class="">
>>> +<br class="">
>>> /// DecodePSHUFMask - This decodes the shuffle masks for pshufd, and vpermilp*.<br class="">
>>> /// VT indicates the type of the vector allowing it to handle different<br class="">
>>> /// datatypes and vector widths.<br class="">
>>><br class="">
>>> Modified: llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h<br class="">
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h?rev=173572&r1=173571&r2=173572&view=diff" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h?rev=173572&r1=173571&r2=173572&view=diff</a><br class="">
>>> ==============================================================================<br class="">
>>> --- llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h (original)<br class="">
>>> +++ llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h Sat Jan 26 07:31:37 2013<br class="">
>>> @@ -35,6 +35,8 @@ void DecodeMOVHLPSMask(unsigned NElts, S<br class="">
>>> // <0,2> or <0,1,4,5><br class="">
>>> void DecodeMOVLHPSMask(unsigned NElts, SmallVectorImpl<int> &ShuffleMask);<br class="">
>>><br class="">
>>> +void DecodePALIGNMask(MVT VT, unsigned Imm, SmallVectorImpl<int> &ShuffleMask);<br class="">
>>> +<br class="">
>>> void DecodePSHUFMask(MVT VT, unsigned Imm, SmallVectorImpl<int> &ShuffleMask);<br class="">
>>><br class="">
>>> void DecodePSHUFHWMask(MVT VT, unsigned Imm, SmallVectorImpl<int> &ShuffleMask);<br class="">
>>><br class="">
>>> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp<br class="">
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=173572&r1=173571&r2=173572&view=diff" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=173572&r1=173571&r2=173572&view=diff</a><br class="">
>>> ==============================================================================<br class="">
>>> --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)<br class="">
>>> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Sat Jan 26 07:31:37 2013<br class="">
>>> @@ -4592,6 +4592,10 @@ static bool getTargetShuffleMask(SDNode<br class="">
>>> case X86ISD::MOVLHPS:<br class="">
>>> DecodeMOVLHPSMask(NumElems, Mask);<br class="">
>>> break;<br class="">
>>> + case X86ISD::PALIGN:<br class="">
>>> + ImmN = N->getOperand(N->getNumOperands()-1);<br class="">
>>> + DecodePALIGNMask(VT, cast<ConstantSDNode>(ImmN)->getZExtValue(), Mask);<br class="">
>>> + break;<br class="">
>>> case X86ISD::PSHUFD:<br class="">
>>> case X86ISD::VPERMILP:<br class="">
>>> ImmN = N->getOperand(N->getNumOperands()-1);<br class="">
>>> @@ -4635,7 +4639,6 @@ static bool getTargetShuffleMask(SDNode<br class="">
>>> case X86ISD::MOVLPS:<br class="">
>>> case X86ISD::MOVSHDUP:<br class="">
>>> case X86ISD::MOVSLDUP:<br class="">
>>> - case X86ISD::PALIGN:<br class="">
>>> // Not yet implemented<br class="">
>>> return false;<br class="">
>>> default: llvm_unreachable("unknown target shuffle node");<br class="">
>>><br class="">
>>> Added: llvm/trunk/test/MC/X86/shuffle-comments.s<br class="">
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/shuffle-comments.s?rev=173572&view=auto" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/shuffle-comments.s?rev=173572&view=auto</a><br class="">
>>> ==============================================================================<br class="">
>>> --- llvm/trunk/test/MC/X86/shuffle-comments.s (added)<br class="">
>>> +++ llvm/trunk/test/MC/X86/shuffle-comments.s Sat Jan 26 07:31:37 2013<br class="">
>>> @@ -0,0 +1,31 @@<br class="">
>>> +# RUN: llvm-mc %s -triple=x86_64-unknown-unknown | FileCheck %s<br class="">
>>> +<br class="">
>>> +palignr $8, %xmm0, %xmm1<br class="">
>>> +# CHECK: xmm1 = xmm0[8,9,10,11,12,13,14,15],xmm1[0,1,2,3,4,5,6,7]<br class="">
>>> +palignr $8, (%rax), %xmm1<br class="">
>>> +# CHECK: xmm1 = mem[8,9,10,11,12,13,14,15],xmm1[0,1,2,3,4,5,6,7]<br class="">
>>> +<br class="">
>>> +palignr $16, %xmm0, %xmm1<br class="">
>>> +# CHECK: xmm1 = xmm1[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]<br class="">
>>> +palignr $16, (%rax), %xmm1<br class="">
>>> +# CHECK: xmm1 = xmm1[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]<br class="">
>>> +<br class="">
>>> +palignr $0, %xmm0, %xmm1<br class="">
>>> +# CHECK: xmm1 = xmm0[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]<br class="">
>>> +palignr $0, (%rax), %xmm1<br class="">
>>> +# CHECK: xmm1 = mem[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]<br class="">
>>> +<br class="">
>>> +vpalignr $8, %xmm0, %xmm1, %xmm2<br class="">
>>> +# CHECK: xmm2 = xmm0[8,9,10,11,12,13,14,15],xmm1[0,1,2,3,4,5,6,7]<br class="">
>>> +vpalignr $8, (%rax), %xmm1, %xmm2<br class="">
>>> +# CHECK: xmm2 = mem[8,9,10,11,12,13,14,15],xmm1[0,1,2,3,4,5,6,7]<br class="">
>>> +<br class="">
>>> +vpalignr $16, %xmm0, %xmm1, %xmm2<br class="">
>>> +# CHECK: xmm2 = xmm1[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]<br class="">
>>> +vpalignr $16, (%rax), %xmm1, %xmm2<br class="">
>>> +# CHECK: xmm2 = xmm1[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]<br class="">
>>> +<br class="">
>>> +vpalignr $0, %xmm0, %xmm1, %xmm2<br class="">
>>> +# CHECK: xmm2 = xmm0[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]<br class="">
>>> +vpalignr $0, (%rax), %xmm1, %xmm2<br class="">
>>> +# CHECK: xmm2 = mem[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]<br class="">
>>><br class="">
>>><br class="">
>>> _______________________________________________<br class="">
>>> llvm-commits mailing list<br class="">
>>> <a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><br class="">
>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br class="">
><br class="">
<br class="">
_______________________________________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><br class="">
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br class="">
</div></div></blockquote></div><br class=""><br clear="all" class=""><div class=""><br class=""></div>-- <br class=""><div class="gmail_signature">~Craig</div>
</div>
</div></blockquote></div><br class=""></body></html>