<div dir="ltr">Thanks, Alexey. Nadav reviewed the patch outside of Phab, so I just made a note of that in the commit message. I'll look into the crash soon.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Dec 30, 2014 at 5:48 PM, Alexey Samsonov <span dir="ltr"><<a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Sorry, I had to revert this commit in r225031.<div><br></div><div>This commit was sent for review in <a href="http://reviews.llvm.org/D6698" target="_blank">http://reviews.llvm.org/D6698</a>, but was later submitted without any comments from reviewers.</div><div>Maybe, that review thread is a good place to get feedback and discuss the correct fix.</div><div><br></div><div>Repro instructions:</div><div>$ cat tmp/b.c</div><div><div>__extension__ typedef unsigned long long int uint64_t;</div><div><br></div><div>#define NLIMBS 9</div><div>typedef uint64_t felem[NLIMBS];</div><div>uint64_t mask;</div><div><br></div><div>void select_point(felem out[4]);</div><div><br></div><div>void batch_mul() {</div><div> felem tmp[4];</div><div> for (int i = 520; i >= 0; --i) {</div><div> select_point(tmp);</div><div> {</div><div> uint64_t *out = tmp[3];</div><div> const uint64_t *in = tmp[1];</div><div> static const uint64_t two62m3 = -1U << 5;</div><div> static const uint64_t two62m2 = -1U << 4;</div><div> out[0] = two62m3 - in[0];</div><div> out[1] = two62m2 - in[1];</div><div> out[2] = two62m2 - in[2];</div><div> out[3] = two62m2 - in[3];</div><div> out[4] = two62m2 - in[4];</div><div> out[5] = two62m2 - in[5];</div><div> out[6] = two62m2 - in[6];</div><div> out[7] = two62m2 - in[7];</div><div> out[8] = two62m2 - in[8];</div><div> }</div><div><br></div><div> {</div><div> uint64_t *out = tmp[1];</div><div> const uint64_t *in = tmp[3];</div><div><br></div><div> for (unsigned int i = 0; i < NLIMBS; ++i) {</div><div> const uint64_t tmp = mask & (in[i] ^ out[i]);</div><div> out[i] ^= tmp;</div><div> }</div><div> }</div><div> }</div><div>}</div></div><div><br></div><div><br></div><div><div>Operand not processed?</div><div>0x5c4c770: v2i64,ch = load 0x5c4ac60, 0x5c4c660, 0x5c45850<LD16[%sunkaddr115](tbaa=<0x5b6c2a8>)> [ORD=61] [ID=1]</div><div><br></div><div>UNREACHABLE executed at lib/CodeGen/SelectionDAG/LegalizeTypes.cpp:438!</div><div>#0 0x14b8498 llvm::sys::PrintStackTrace(_IO_FILE*) (bin/clang-3.6+0x14b8498)</div><div>#1 0x14b9a2b SignalHandler(int) (bin/clang-3.6+0x14b9a2b)</div><div>#2 0x7f11fbfff340 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x10340)</div><div>#3 0x7f11fb220bb9 gsignal /build/buildd/eglibc-2.19/signal/../nptl/sysdeps/unix/sysv/linux/raise.c:56:0</div><div>#4 0x7f11fb223fc8 abort /build/buildd/eglibc-2.19/stdlib/abort.c:91:0</div><div>#5 0x1484373 llvm::llvm_unreachable_internal(char const*, char const*, unsigned int) (bin/clang-3.6+0x1484373)</div><div>#6 0x1885dce llvm::DAGTypeLegalizer::run() (bin/clang-3.6+0x1885dce)</div><div>#7 0x188b294 llvm::SelectionDAG::LegalizeTypes() (bin/clang-3.6+0x188b294)</div><div>#8 0x184c6c7 llvm::SelectionDAGISel::CodeGenAndEmitDAG() (bin/clang-3.6+0x184c6c7)</div><div>#9 0x184b9b8 llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) (bin/clang-3.6+0x184b9b8)</div><div>#10 0x1848634 llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) (bin/clang-3.6+0x1848634)</div><div>#11 0xc85a96 (anonymous namespace)::X86DAGToDAGISel::runOnMachineFunction(llvm::MachineFunction&) (bin/clang-3.6+0xc85a96)</div><div>#12 0xf7f5bc llvm::MachineFunctionPass::runOnFunction(llvm::Function&) (bin/clang-3.6+0xf7f5bc)</div><div>#13 0x11b53f3 llvm::FPPassManager::runOnFunction(llvm::Function&) (bin/clang-3.6+0x11b53f3)</div><div>#14 0x11b566b llvm::FPPassManager::runOnModule(llvm::Module&) (bin/clang-3.6+0x11b566b)</div><div>#15 0x11b5bc7 llvm::legacy::PassManagerImpl::run(llvm::Module&) (bin/clang-3.6+0x11b5bc7)</div><div>#16 0x1905f5d clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::StringRef, llvm::Module*, clang::BackendAction, llvm::raw_ostream*) (bin/clang-3.6+0x1905f5d)</div><div>#17 0x18fab95 clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) (bin/clang-3.6+0x18fab95)</div><div>#18 0x1e8c793 clang::ParseAST(clang::Sema&, bool, bool) (bin/clang-3.6+0x1e8c793)</div><div>#19 0x166104e clang::FrontendAction::Execute() (bin/clang-3.6+0x166104e)</div><div>#20 0x163207c clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (bin/clang-3.6+0x163207c)</div><div>#21 0x16e561a clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (bin/clang-3.6+0x16e561a)</div><div>#22 0x6ca3d2 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (bin/clang-3.6+0x6ca3d2)</div><div>#23 0x6c914a main (bin/clang-3.6+0x6c914a)</div><div>#24 0x7f11fb20bec5 __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:321:0</div><div>#25 0x6c612b _start (bin/clang-3.6+0x6c612b)</div><div>Stack dump:</div><div>0.<span style="white-space:pre-wrap"> </span>Program arguments: bin/clang -cc1 -triple x86_64-unknown-linux-gnu -emit-obj -mnoexecstack -main-file-name b.c -mrelocation-model pic -pic-level 2 -mthread-model posix -mdisable-fp-elim -target-cpu x86-64 -target-feature +sse3 -ffunction-sections -fdata-sections -O2 -ferror-limit 19 -fmessage-length 0 -mstackrealign -fno-signed-char -fobjc-runtime=gcc -fdiagnostics-show-option -fcolor-diagnostics -vectorize-loops -vectorize-slp -x c tmp/b.c </div><div>1.<span style="white-space:pre-wrap"> </span><eof> parser at end of file</div><div>2.<span style="white-space:pre-wrap"> </span>Code generation</div><div>3.<span style="white-space:pre-wrap"> </span>Running pass 'Function Pass Manager' on module 'tmp/b.c'.</div><div>4.<span style="white-space:pre-wrap"> </span>Running pass 'X86 DAG->DAG Instruction Selection' on function '@batch_mul'</div></div><div><br></div></div><div class="gmail_extra"><div><div class="h5"><br><div class="gmail_quote">On Fri, Dec 19, 2014 at 12:23 PM, Sanjay Patel <span dir="ltr"><<a href="mailto:spatel@rotateright.com" target="_blank">spatel@rotateright.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: spatel<br>
Date: Fri Dec 19 14:23:41 2014<br>
New Revision: 224611<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=224611&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=224611&view=rev</a><br>
Log:<br>
merge consecutive stores of extracted vector elements<br>
<br>
Add a path to DAGCombiner::MergeConsecutiveStores()<br>
to combine multiple scalar stores when the store operands<br>
are extracted vector elements. This is a partial fix for<br>
PR21711 ( <a href="http://llvm.org/bugs/show_bug.cgi?id=21711" target="_blank">http://llvm.org/bugs/show_bug.cgi?id=21711</a> ).<br>
<br>
For the new test case, codegen improves from:<br>
<br>
vmovss %xmm0, (%rdi)<br>
vextractps $1, %xmm0, 4(%rdi)<br>
vextractps $2, %xmm0, 8(%rdi)<br>
vextractps $3, %xmm0, 12(%rdi)<br>
vextractf128 $1, %ymm0, %xmm0<br>
vmovss %xmm0, 16(%rdi)<br>
vextractps $1, %xmm0, 20(%rdi)<br>
vextractps $2, %xmm0, 24(%rdi)<br>
vextractps $3, %xmm0, 28(%rdi)<br>
vzeroupper<br>
retq<br>
<br>
To:<br>
<br>
vmovups %ymm0, (%rdi)<br>
vzeroupper<br>
retq<br>
<br>
Patch reviewed by Nadav Rotem.<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D6698" target="_blank">http://reviews.llvm.org/D6698</a><br>
<br>
<br>
Modified:<br>
llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br>
llvm/trunk/test/CodeGen/X86/MergeConsecutiveStores.ll<br>
<br>
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=224611&r1=224610&r2=224611&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=224611&r1=224610&r2=224611&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Fri Dec 19 14:23:41 2014<br>
@@ -9498,11 +9498,14 @@ bool DAGCombiner::MergeConsecutiveStores<br>
return false;<br>
<br>
// Perform an early exit check. Do not bother looking at stored values that<br>
- // are not constants or loads.<br>
+ // are not constants, loads, or extracted vector elements.<br>
SDValue StoredVal = St->getValue();<br>
bool IsLoadSrc = isa<LoadSDNode>(StoredVal);<br>
- if (!isa<ConstantSDNode>(StoredVal) && !isa<ConstantFPSDNode>(StoredVal) &&<br>
- !IsLoadSrc)<br>
+ bool IsConstantSrc = isa<ConstantSDNode>(StoredVal) ||<br>
+ isa<ConstantFPSDNode>(StoredVal);<br>
+ bool IsExtractVecEltSrc = (StoredVal.getOpcode() == ISD::EXTRACT_VECTOR_ELT);<br>
+<br>
+ if (!IsConstantSrc && !IsLoadSrc && !IsExtractVecEltSrc)<br>
return false;<br>
<br>
// Only look at ends of store sequences.<br>
@@ -9644,7 +9647,7 @@ bool DAGCombiner::MergeConsecutiveStores<br>
LSBaseSDNode *FirstInChain = StoreNodes[0].MemNode;<br>
<br>
// Store the constants into memory as one consecutive store.<br>
- if (!IsLoadSrc) {<br>
+ if (IsConstantSrc) {<br>
unsigned LastLegalType = 0;<br>
unsigned LastLegalVectorType = 0;<br>
bool NonZero = false;<br>
@@ -9769,6 +9772,74 @@ bool DAGCombiner::MergeConsecutiveStores<br>
while (!St->use_empty())<br>
DAG.ReplaceAllUsesWith(SDValue(St, 0), St->getChain());<br>
deleteAndRecombine(St);<br>
+ }<br>
+<br>
+ return true;<br>
+ }<br>
+<br>
+ // When extracting multiple vector elements, try to store them<br>
+ // in one vector store rather than a sequence of scalar stores.<br>
+ if (IsExtractVecEltSrc) {<br>
+ unsigned NumElem = 0;<br>
+ for (unsigned i = 0; i < LastConsecutiveStore + 1; ++i) {<br>
+ // Find a legal type for the vector store.<br>
+ EVT Ty = EVT::getVectorVT(*DAG.getContext(), MemVT, i+1);<br>
+ if (TLI.isTypeLegal(Ty))<br>
+ NumElem = i + 1;<br>
+ }<br>
+<br>
+ // Make sure we have a legal type and something to merge.<br>
+ if (NumElem < 2)<br>
+ return false;<br>
+<br>
+ unsigned EarliestNodeUsed = 0;<br>
+ for (unsigned i=0; i < NumElem; ++i) {<br>
+ // Find a chain for the new wide-store operand. Notice that some<br>
+ // of the store nodes that we found may not be selected for inclusion<br>
+ // in the wide store. The chain we use needs to be the chain of the<br>
+ // earliest store node which is *used* and replaced by the wide store.<br>
+ if (StoreNodes[i].SequenceNum > StoreNodes[EarliestNodeUsed].SequenceNum)<br>
+ EarliestNodeUsed = i;<br>
+ }<br>
+<br>
+ // The earliest Node in the DAG.<br>
+ LSBaseSDNode *EarliestOp = StoreNodes[EarliestNodeUsed].MemNode;<br>
+ SDLoc DL(StoreNodes[0].MemNode);<br>
+<br>
+ SDValue StoredVal;<br>
+<br>
+ // Find a legal type for the vector store.<br>
+ EVT Ty = EVT::getVectorVT(*DAG.getContext(), MemVT, NumElem);<br>
+<br>
+ SmallVector<SDValue, 8> Ops;<br>
+ for (unsigned i = 0; i < NumElem ; ++i) {<br>
+ StoreSDNode *St = cast<StoreSDNode>(StoreNodes[i].MemNode);<br>
+ SDValue Val = St->getValue();<br>
+ // All of the operands of a BUILD_VECTOR must have the same type.<br>
+ if (Val.getValueType() != MemVT)<br>
+ return false;<br>
+ Ops.push_back(Val);<br>
+ }<br>
+<br>
+ // Build the extracted vector elements back into a vector.<br>
+ StoredVal = DAG.getNode(ISD::BUILD_VECTOR, DL, Ty, Ops);<br>
+<br>
+ SDValue NewStore = DAG.getStore(EarliestOp->getChain(), DL, StoredVal,<br>
+ FirstInChain->getBasePtr(),<br>
+ FirstInChain->getPointerInfo(),<br>
+ false, false,<br>
+ FirstInChain->getAlignment());<br>
+<br>
+ // Replace the first store with the new store<br>
+ CombineTo(EarliestOp, NewStore);<br>
+ // Erase all other stores.<br>
+ for (unsigned i = 0; i < NumElem ; ++i) {<br>
+ if (StoreNodes[i].MemNode == EarliestOp)<br>
+ continue;<br>
+ StoreSDNode *St = cast<StoreSDNode>(StoreNodes[i].MemNode);<br>
+ while (!St->use_empty())<br>
+ DAG.ReplaceAllUsesWith(SDValue(St, 0), St->getChain());<br>
+ deleteAndRecombine(St);<br>
}<br>
<br>
return true;<br>
<br>
Modified: llvm/trunk/test/CodeGen/X86/MergeConsecutiveStores.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/MergeConsecutiveStores.ll?rev=224611&r1=224610&r2=224611&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/MergeConsecutiveStores.ll?rev=224611&r1=224610&r2=224611&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/CodeGen/X86/MergeConsecutiveStores.ll (original)<br>
+++ llvm/trunk/test/CodeGen/X86/MergeConsecutiveStores.ll Fri Dec 19 14:23:41 2014<br>
@@ -434,3 +434,36 @@ define void @loadStoreBaseIndexOffsetSex<br>
; <label>:14<br>
ret void<br>
}<br>
+<br>
+define void @merge_vec_element_store(<8 x float> %v, float* %ptr) {<br>
+ %vecext0 = extractelement <8 x float> %v, i32 0<br>
+ %vecext1 = extractelement <8 x float> %v, i32 1<br>
+ %vecext2 = extractelement <8 x float> %v, i32 2<br>
+ %vecext3 = extractelement <8 x float> %v, i32 3<br>
+ %vecext4 = extractelement <8 x float> %v, i32 4<br>
+ %vecext5 = extractelement <8 x float> %v, i32 5<br>
+ %vecext6 = extractelement <8 x float> %v, i32 6<br>
+ %vecext7 = extractelement <8 x float> %v, i32 7<br>
+ %arrayidx1 = getelementptr inbounds float* %ptr, i64 1<br>
+ %arrayidx2 = getelementptr inbounds float* %ptr, i64 2<br>
+ %arrayidx3 = getelementptr inbounds float* %ptr, i64 3<br>
+ %arrayidx4 = getelementptr inbounds float* %ptr, i64 4<br>
+ %arrayidx5 = getelementptr inbounds float* %ptr, i64 5<br>
+ %arrayidx6 = getelementptr inbounds float* %ptr, i64 6<br>
+ %arrayidx7 = getelementptr inbounds float* %ptr, i64 7<br>
+ store float %vecext0, float* %ptr, align 4<br>
+ store float %vecext1, float* %arrayidx1, align 4<br>
+ store float %vecext2, float* %arrayidx2, align 4<br>
+ store float %vecext3, float* %arrayidx3, align 4<br>
+ store float %vecext4, float* %arrayidx4, align 4<br>
+ store float %vecext5, float* %arrayidx5, align 4<br>
+ store float %vecext6, float* %arrayidx6, align 4<br>
+ store float %vecext7, float* %arrayidx7, align 4<br>
+ ret void<br>
+<br>
+; CHECK-LABEL: merge_vec_element_store<br>
+; CHECK: vmovups<br>
+; CHECK-NEXT: vzeroupper<br>
+; CHECK-NEXT: retq<br>
+}<br>
+<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></div><br><br clear="all"><div><br></div></div></div><span class="HOEnZb"><font color="#888888">-- <br><div><div dir="ltr">Alexey Samsonov<br><a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a></div></div>
</font></span></div>
</blockquote></div><br></div>