<div dir="ltr">I just hadn't seen this email in eons...<br><br>I have absolutely no ideas about how to fix this.... yuck.<br><div><br></div><div>I'll have to spend some time poking around at things to even make heads or tails of it.</div></div><br><div class="gmail_quote">On Thu, May 28, 2015 at 4:32 PM Pete Cooper <<a href="mailto:peter_cooper@apple.com">peter_cooper@apple.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">Ping.<div><br></div><div>Chandler, this is a pretty bad layering violation. I’d really like to come up with a good solution for fixing it.</div><div><br></div><div>And i know you’re interested in this kind of thing because you said so yesterday ;) (<a href="http://lists.cs.uiuc.edu/pipermail/llvmdev/2015-May/086127.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/llvmdev/2015-May/086127.html</a>)</div><div><br></div><div>Cheers,</div><div>Pete</div></div><div style="word-wrap:break-word"><div><br><div><div><blockquote type="cite"><div>On May 14, 2015, at 11:03 AM, Pete Cooper <<a href="mailto:peter_cooper@apple.com" target="_blank">peter_cooper@apple.com</a>> wrote:</div><br><div><div>Ping<br><blockquote type="cite">On Apr 20, 2015, at 3:18 PM, Pete Cooper <<a href="mailto:peter_cooper@apple.com" target="_blank">peter_cooper@apple.com</a>> wrote:<br><br>Hi Chandler<br><br>Sorry to resurrect an old thread.<br><br>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.<br><br>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.<br><br>Can you think of any better solutions? I’m happy to do the work if we can come up with the right answer.<br><br>Cheers,<br>Pete<br><blockquote type="cite">On Jul 25, 2014, at 4:47 PM, Chandler Carruth <<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>> wrote:<br><br>Author: chandlerc<br>Date: Fri Jul 25 18:47:11 2014<br>New Revision: 213986<br><br>URL: <a href="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=" target="_blank">http://llvm.org/viewvc/llvm-project?rev=213986&view=rev</a><br>Log:<br>[x86] Teach the X86 backend to print shuffle comments for PSHUFB<br>instructions which happen to have a constant mask.<br><br>Currently, this only handles a very narrow set of cases, but those<br>happen to be the cases that I care about for testing shuffles sanely.<br>This is a bit trickier than other shuffle instructions because we're<br>decoding constants out of the constant pool. The current MC layer makes<br>it completely impossible to inspect a constant pool entry, so we have to<br>do it at the MI level and attach the comment to the streamer on its way<br>out. So no joy for disassembling, but it does make test cases and asm<br>dumps *much* nicer.<br><br>Sorry for no test cases, but it didn't really seem that valuable to go<br>trolling through existing old test cases and updating them. I'll have<br>lots of testing of this in the upcoming patch for SSSE3 emission in the<br>new vector shuffle lowering code paths.<br><br>Modified:<br> llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp<br> llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h<br> llvm/trunk/lib/Target/X86/X86MCInstLower.cpp<br><br>Modified: llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp<br>URL: <a href="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=" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp?rev=213986&r1=213985&r2=213986&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp (original)<br>+++ llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.cpp Fri Jul 25 18:47:11 2014<br>@@ -13,6 +13,7 @@<br>//===----------------------------------------------------------------------===//<br><br>#include "X86ShuffleDecode.h"<br>+#include "llvm/IR/Constants.h"<br>#include "llvm/CodeGen/MachineValueType.h"<br><br>//===----------------------------------------------------------------------===//<br>@@ -207,6 +208,38 @@ void DecodeVPERM2X128Mask(MVT VT, unsign<br> }<br>}<br><br>+/// \brief Decode PSHUFB masks stored in an LLVM Constant.<br>+void DecodePSHUFBMask(const ConstantDataSequential *C,<br>+ SmallVectorImpl<int> &ShuffleMask) {<br>+ Type *MaskTy = C->getType();<br>+ assert(MaskTy->isVectorTy() && "Expected a vector constant mask!");<br>+ Type *EltTy = MaskTy->getVectorElementType();<br>+ assert(EltTy->isIntegerTy(8) && "Expected i8 constant mask elements!");<br>+ int NumElements = MaskTy->getVectorNumElements();<br>+ // FIXME: Add support for AVX-512.<br>+ assert((NumElements == 16 || NumElements == 32) &&<br>+ "Only 128-bit and 256-bit vectors supported!");<br>+ assert((unsigned)NumElements == C->getNumElements() &&<br>+ "Constant mask has a different number of elements!");<br>+<br>+ ShuffleMask.reserve(NumElements);<br>+ for (int i = 0; i < NumElements; ++i) {<br>+ // For AVX vectors with 32 bytes the base of the shuffle is the half of the<br>+ // vector we're inside.<br>+ int Base = i < 16 ? 0 : 16;<br>+ uint64_t Element = C->getElementAsInteger(i);<br>+ // If the high bit (7) of the byte is set, the element is zeroed.<br>+ if (Element & (1 << 7))<br>+ ShuffleMask.push_back(SM_SentinelZero);<br>+ else {<br>+ int Index = Base + Element;<br>+ assert((Index >= 0 && Index < NumElements) ||<br>+ "Out of bounds shuffle index for pshub instruction!");<br>+ ShuffleMask.push_back(Index);<br>+ }<br>+ }<br>+}<br>+<br>/// DecodeVPERMMask - this decodes the shuffle masks for VPERMQ/VPERMPD.<br>/// No VT provided since it only works on 256-bit, 4 element vectors.<br>void DecodeVPERMMask(unsigned Imm, SmallVectorImpl<int> &ShuffleMask) {<br><br>Modified: llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h<br>URL: <a href="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=" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h?rev=213986&r1=213985&r2=213986&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h (original)<br>+++ llvm/trunk/lib/Target/X86/Utils/X86ShuffleDecode.h Fri Jul 25 18:47:11 2014<br>@@ -22,6 +22,7 @@<br>//===----------------------------------------------------------------------===//<br><br>namespace llvm {<br>+class ConstantDataSequential;<br>class MVT;<br><br>enum {<br>@@ -59,6 +60,8 @@ void DecodeUNPCKHMask(MVT VT, SmallVecto<br>/// different datatypes and vector widths.<br>void DecodeUNPCKLMask(MVT VT, SmallVectorImpl<int> &ShuffleMask);<br><br>+void DecodePSHUFBMask(const ConstantDataSequential *C,<br>+ SmallVectorImpl<int> &ShuffleMask);<br><br>void DecodeVPERM2X128Mask(MVT VT, unsigned Imm,<br> SmallVectorImpl<int> &ShuffleMask);<br><br>Modified: llvm/trunk/lib/Target/X86/X86MCInstLower.cpp<br>URL: <a href="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=" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86MCInstLower.cpp?rev=213986&r1=213985&r2=213986&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/Target/X86/X86MCInstLower.cpp (original)<br>+++ llvm/trunk/lib/Target/X86/X86MCInstLower.cpp Fri Jul 25 18:47:11 2014<br>@@ -16,8 +16,11 @@<br>#include "X86RegisterInfo.h"<br>#include "InstPrinter/X86ATTInstPrinter.h"<br>#include "MCTargetDesc/X86BaseInfo.h"<br>+#include "Utils/X86ShuffleDecode.h"<br>#include "llvm/ADT/SmallString.h"<br>#include "llvm/CodeGen/MachineFunction.h"<br>+#include "llvm/CodeGen/MachineConstantPool.h"<br>+#include "llvm/CodeGen/MachineOperand.h"<br>#include "llvm/CodeGen/MachineModuleInfoImpls.h"<br>#include "llvm/CodeGen/StackMaps.h"<br>#include "llvm/IR/DataLayout.h"<br>@@ -963,6 +966,83 @@ void X86AsmPrinter::EmitInstruction(cons<br> case X86::SEH_EndPrologue:<br> OutStreamer.EmitWinCFIEndProlog();<br> return;<br>+<br>+ case X86::PSHUFBrm:<br>+ // Lower PSHUFB normally but add a comment if we can find a constant<br>+ // shuffle mask. We won't be able to do this at the MC layer because the<br>+ // mask isn't an immediate.<br>+ std::string Comment;<br>+ raw_string_ostream CS(Comment);<br>+ SmallVector<int, 16> Mask;<br>+<br>+ assert(MI->getNumOperands() == 7 &&<br>+ "Wrong number of operansd for PSHUFBrm");<br>+ const MachineOperand &DstOp = MI->getOperand(0);<br>+ const MachineOperand &SrcOp = MI->getOperand(1);<br>+ const MachineOperand &MaskOp = MI->getOperand(5);<br>+<br>+ // Compute the name for a register. This is really goofy because we have<br>+ // multiple instruction printers that could (in theory) use different<br>+ // names. Fortunately most people use the ATT style (outside of Windows)<br>+ // and they actually agree on register naming here. Ultimately, this is<br>+ // a comment, and so its OK if it isn't perfect.<br>+ auto GetRegisterName = [](unsigned RegNum) -> StringRef {<br>+ return X86ATTInstPrinter::getRegisterName(RegNum);<br>+ };<br>+<br>+ StringRef DstName = DstOp.isReg() ? GetRegisterName(DstOp.getReg()) : "mem";<br>+ StringRef SrcName = SrcOp.isReg() ? GetRegisterName(SrcOp.getReg()) : "mem";<br>+ CS << DstName << " = ";<br>+<br>+ if (MaskOp.isCPI()) {<br>+ ArrayRef<MachineConstantPoolEntry> Constants =<br>+ MI->getParent()->getParent()->getConstantPool()->getConstants();<br>+ const MachineConstantPoolEntry &MaskConstantEntry =<br>+ Constants[MI->getOperand(5).getIndex()];<br>+ Type *MaskTy = MaskConstantEntry.getType();<br>+ if (!MaskConstantEntry.isMachineConstantPoolEntry())<br>+ if (auto *C = dyn_cast<ConstantDataSequential>(<br>+ MaskConstantEntry.Val.ConstVal)) {<br>+ assert(MaskTy == C->getType() &&<br>+ "Expected a constant of the same type!");<br>+<br>+ DecodePSHUFBMask(C, Mask);<br>+ assert(Mask.size() == MaskTy->getVectorNumElements() &&<br>+ "Shuffle mask has a different size than its type!");<br>+ }<br>+ }<br>+<br>+ if (!Mask.empty()) {<br>+ bool NeedComma = false;<br>+ bool InSrc = false;<br>+ for (int M : Mask) {<br>+ // Wrap up any prior entry...<br>+ if (M == SM_SentinelZero && InSrc) {<br>+ InSrc = false;<br>+ CS << "]";<br>+ }<br>+ if (NeedComma)<br>+ CS << ",";<br>+ else<br>+ NeedComma = true;<br>+<br>+ // Print this shuffle...<br>+ if (M == SM_SentinelZero) {<br>+ CS << "zero";<br>+ } else {<br>+ if (!InSrc) {<br>+ InSrc = true;<br>+ CS << SrcName << "[";<br>+ }<br>+ CS << M;<br>+ }<br>+ }<br>+ if (InSrc)<br>+ CS << "]";<br>+<br>+ OutStreamer.AddComment(CS.str());<br>+ }<br>+ break;<br> }<br><br> MCInst TmpInst;<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></blockquote><br></blockquote><br></div></div></blockquote></div><br></div></div></div>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>