[PATCH] D42989: [X86] When doing callee save/restore for k-registers make sure we don't use KMOVQ on non-BWI targets

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Target/X86/X86InstrInfo.cpp:6922
       return load ? X86::LD_Fp32m : X86::ST_Fp32m;
-    if (X86::VK32RegClass.hasSubClassEq(RC))
+    if (X86::VK32RegClass.hasSubClassEq(RC)) {
+      assert(STI.hasBWI() && "KMOVD requires BWI");
----------------
delena wrote:
> craig.topper wrote:
> > delena wrote:
> > > if you'll check STI and choose KMOVW here instead of KMOVD it will be enough.
> > I considered that but it would also hide PR36254 and maybe other future bugs. So I thought it was best to try to fix the issue closer to its source.
> But we do this for all other cases in this function - we check target and generate the right instruction.
> IMO, accumulating logic in one place is always better than spreading it over the code.
None of the cases are changing the width of the save (except maybe BNDRRegClass). If we use KMOVW here we're saving 2 bytes into an 8 byte spill slot. With the fix in this patch we get the correct spill size.

The memory operand on the instruction would still show 8 bytes. It will print "8-byte spill" in the debug output. If we had any instructions that allowed load folding of k-registers(I know we don't) would it mess those up too?


https://reviews.llvm.org/D42989