Hi, I was build SSE performance work on mac intel. But I found the performance of my SSE4.1 version code using in xcode 12.4 is not as good as xcode 10.1, so I checked the assembly of my code. The one _mm_mul_epi() instruction was translated into three pmuludq, which is the SSE2 instruction, while this was normal when compiling on xcode 10.1 and _mm_mul_epi() was translated into pmuldq. So I checked the clang version, and I found this error occured when clang version larger than 7.0.0. The simple case can be found: https://godbolt.org/z/Tf7qeocvz I probably think this is a clang compiler bug. And I hope to get some advice on how to solve this issue. Thanks. Best, Wade
There's something very strange going on in the IR out of the middle end %37 = shl <2 x i64> %24, <i64 32, i64 32>, !dbg !327 %38 = ashr exact <2 x i64> %37, <i64 32, i64 32>, !dbg !327 %39 = shl <2 x i64> %36, <i64 32, i64 32>, !dbg !327 %40 = ashr exact <2 x i64> %39, <i64 32, i64 32>, !dbg !327 %41 = mul nsw <2 x i64> %40, %38, !dbg !327 I assume InstCombine has gotten clever, but I'm not sure why yet.
Disregard that last comment. That's our IR for pmuldq that we should pattern match. Will keep digging.
I think because warping is loop invariant, it got pulled out of the loop along with the _mm_set1 and the 2xi64 shl+ashr we insert to sign extend the mul input for pmuldq pattern match. Because we don't have a vsraq instruction we emit this pshufd $0, %xmm0, %xmm0 # xmm0 = xmm0[0,0,0,0] movdqa %xmm0, %xmm1 psllq $32, %xmm1 psrad $31, %xmm1 pblendw $51, %xmm0, %xmm1 # xmm1 = xmm0[0,1],xmm1[2,3],xmm0[4,5],xmm1[6,7] And we aren't able to create an AssertSExt from that to tell the block inside the loop that the input is already sign extended to allow pmuldq to be match. And the sign extend outside the loop would have been unnecessary if we formed pmuldq.
Maybe we need an X86CodeGenPrepare to bring them back into the same basic block? I assume the same thing can happen to pmuludq.
(In reply to Craig Topper from comment #3) > I think because warping is loop invariant, it got pulled out of the loop > along with the _mm_set1 and the 2xi64 shl+ashr we insert to sign extend the > mul input for pmuldq pattern match. > > Because we don't have a vsraq instruction we emit this > > pshufd $0, %xmm0, %xmm0 # xmm0 = xmm0[0,0,0,0] > > movdqa %xmm0, %xmm1 > > psllq $32, %xmm1 > > psrad $31, %xmm1 > > pblendw $51, %xmm0, %xmm1 # xmm1 = > xmm0[0,1],xmm1[2,3],xmm0[4,5],xmm1[6,7] > > And we aren't able to create an AssertSExt from that to tell the block > inside the loop that the input is already sign extended to allow pmuldq to > be match. And the sign extend outside the loop would have been unnecessary > if we formed pmuldq. Thanks for your analysis, and I also want to know is there any solution from code level to avoid this problem?
To fix this in the compiler, I think we just need to add this to X86TargetLowering::shouldSinkOperands I'm not sure how to work around it in code.
Candidate patch https://reviews.llvm.org/D107689
Anything left to do here? Not sure if this qualifies for backport to the release branch.
(In reply to Sanjay Patel from comment #8) > Anything left to do here? > > Not sure if this qualifies for backport to the release branch. I don't think so - its a rather old perf regression. Tentatively resolving this.
(In reply to Simon Pilgrim from comment #9) > (In reply to Sanjay Patel from comment #8) > > Anything left to do here? > > > > Not sure if this qualifies for backport to the release branch. > > I don't think so - its a rather old perf regression. > > Tentatively resolving this. Thank you for resolving this issue, so what's the release version of llvm which can avoid this issue?
(In reply to Wade Wang from comment #10) > (In reply to Simon Pilgrim from comment #9) > > (In reply to Sanjay Patel from comment #8) > > > Anything left to do here? > > > > > > Not sure if this qualifies for backport to the release branch. > > > > I don't think so - its a rather old perf regression. > > > > Tentatively resolving this. > > Thank you for resolving this issue, so what's the release version of llvm > which can avoid this issue? It will be in 14.0 (about 6 months from now).