[llvm-dev] unnecessary reload of 8-byte struct on i386

David Zarzycki via llvm-dev llvm-dev at lists.llvm.org
Mon Oct 28 23:23:21 PDT 2019


This just looks like a temporary stack variable wasn’t properly eliminated because the compiler modeled `Operand` internally as an “i64” which i386 doesn’t natively support. A further reduction, with notes about changes that sidestep the bug:

https://godbolt.org/z/zcCguv



> On Oct 26, 2019, at 12:28 AM, Seth Brenith via llvm-dev <llvm-dev at lists.llvm.org> wrote:
> 
> Hello folks,
>  
> I’ve recently been looking at the generated code for a few functions in Chromium while investigating crashes, and I came across a curious pattern. A smallish repro case is available at https://godbolt.org/z/Dsu1WI <https://godbolt.org/z/Dsu1WI> . In that case, the function Assembler::emit_arith receives a struct (Operand) by value and passes it by value to another function. That struct is 8 bytes long, so the -O3 generated code uses movsd to copy it up the stack. However, we end up with some loads that aren’t needed, as in the following chunk:
>  
> movsd xmm0, qword ptr [ecx] # xmm0 = mem[0],zero
> mov dword ptr [esp + 24], edx
> movsd qword ptr [esp + 40], xmm0
> movsd xmm0, qword ptr [esp + 40] # xmm0 = mem[0],zero
> movsd qword ptr [esp + 8], xmm0
>  
> As far as I can tell, the fourth line has no effect. On its own, that seems like a small missed opportunity for optimization. However, this sequence of instructions also appears to trigger a hardware bug on a small fraction of devices which sometimes end up storing zero at esp+8. A more in-depth discussion of that issue can be found here: https://bugs.chromium.org/p/v8/issues/detail?id=9774 <https://bugs.chromium.org/p/v8/issues/detail?id=9774> .
>  
> I’m hoping that getting rid of the second load in the sequence above would appease these misbehaving machines (though of course I don’t know that it would), as well as making the code a little smaller for everybody else. Does that sound like a reasonable idea? Would LLVM be interested in a patch related to eliminating reloads like this? Does anybody have advice about where I should start looking, or any reasons it would be very hard to achieve the result I’m hoping for?
>  
> Thanks,
> Seth
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20191029/222534ae/attachment-0001.html>


More information about the llvm-dev mailing list