[PATCH] D11477: fix invalid load folding with SSE/AVX FP logical instructions (PR22371)

Sanjay Patel spatel at rotateright.com
Thu Jul 23 15:11:25 PDT 2015


spatel created this revision.
spatel added reviewers: chandlerc, qcolombet, hfinkel.
spatel added a subscriber: llvm-commits.

This is a follow-up to the FIXME that was added with D7474 ( http://reviews.llvm.org/rL229531 ).
I thought this load folding bug had been made hard-to-hit, but it turns out to be very easy when targeting 32-bit x86 and causes a miscompile/crash in Wine:
https://bugs.winehq.org/show_bug.cgi?id=38826
https://llvm.org/bugs/show_bug.cgi?id=22371#c25

The quick fix is to simply remove the scalar FP logical instructions from the load folding table in X86InstrInfo, but that causes us to miss load folds that should be possible when lowering fabs, fneg, fcopysign. So the majority of this patch is altering those lowerings to use *vector* FP logical instructions (because that's all x86 gives us anyway). That lets us do the load folding legally.

The test case for PR2656 was actually checking for miscompiled code, so I changed that. I added the latest test case from PR22371 for extra verification. The changes in sse-fcopysign.ll look benign to me; just different scheduling / RA. I'm not sure why we had 'vandps' and now have 'vandpd' in vec_fabs.ll, but those are logically identical.

http://reviews.llvm.org/D11477

Files:
  lib/Target/X86/X86ISelLowering.cpp
  lib/Target/X86/X86InstrInfo.cpp
  lib/Target/X86/X86InstrSSE.td
  test/CodeGen/X86/pr2656.ll
  test/CodeGen/X86/sse-fcopysign.ll
  test/CodeGen/X86/vec_fabs.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D11477.30525.patch
Type: text/x-patch
Size: 13224 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150723/47a13133/attachment.bin>


More information about the llvm-commits mailing list